[RFC PATCH 0/3] MediaTek Hardware Voter driver

[RFC PATCH 0/3] MediaTek Hardware Voter driver

[AngeloGioacchino Del Regno]

This RFC comes after public discussion [1] with krzk about how the
MediaTek Hardware Voter mechanism was used in the clock driver.

The logic with this implementation instead is to never write to
the HW Voter registers from external drivers, but to register a
driver to handle the voting with internal functions...

...as you can see, I've modeled the power controller to be child
of the HWV, but the clock controller still gets a handle with the
mediatek,hardware-voter property.

The intention for usage of that property now is to get a handle
to the mtk-hardware-voter driver, and call its clock en/dis vote
mtk_hardware_voter_clk_enable() function, which executes a HWV
version-specific callback doing the necessary register programming
for the available HWV version.

I thought about registering the clocks in the HWV and then get
them assigned with something like

	clocks = <&hw_voter CLOCK_HW_IP CLOCK>;

...but I opted for not doing that because the implementation looks
a bit messy as, on MediaTek SoCs, there are multiple clock controllers
located in different IPs, each of them having multiple sets of HWV
registers (from 2 to 8 sets of registers, which are not contiguous
and can't be calculated with offsets), for a total of, well, didn't
really count, but it's more than 80 registers.

Also, all of the mux-gate clocks need to be voted in HWV and after
enablement the "FENC" needs to be brought up with direct MMIO writes
to the clock controller iospace.
(so: enable in HWV -> write FENC enable in clock controller)

For CLOCK_HW_IP in the example, that'd be one of:
vlpckgen, topckgen, imp-iic-wrap-{w,n,s,e,c},mminfra,dispsys0,dispsys1,
ovlsys0,ovlsys1,vencsys,vdecsys (each having dozens of voted clocks).

I'm not sure if I'm forgetting any information but in case just ask....

Ah, "just in case"! ... I've pushed a branch here [2] that contains
the same changes that I'm sending as RFC. Perhaps someone will find
that easier to browse.

Please don't mind about some commits that are not really "presentable"
in that branch (as in, there's some dirty code around) - some stuff is
quite a bit WIP (and some other is quite a bit incomplete).

Also, being this a RFC ... this wasn't tested, it may work or may not,
but that being actually working is out of scope right now, I just want
to understand if I'm walking in the right direction :-)

Cheers!
Angelo

[1]: https://lore.kernel.org/all/20250627-ingenious-tourmaline-wapiti-fa7676@krzk-bin/
[2]: https://gitlab.collabora.com/mediatek/aiot/linux/-/commits/mediatek-hwvoter-temp

AngeloGioacchino Del Regno (3):
  firmware: Move MediaTek ADSP IPC driver to mediatek folder
  dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)
  firmware: mediatek: Add MediaTek Hardware Voter MCU driver

 .../mediatek,mt6991-hardware-voter.yaml       |  70 +++
 drivers/firmware/Kconfig                      |  10 +-
 drivers/firmware/Makefile                     |   2 +-
 drivers/firmware/mediatek/Kconfig             |  23 +
 drivers/firmware/mediatek/Makefile            |   4 +
 .../firmware/{ => mediatek}/mtk-adsp-ipc.c    |   0
 .../firmware/mediatek/mtk-hardware-voter.c    | 430 ++++++++++++++++++
 .../firmware/mediatek/mtk-hardware-voter.h    |  34 ++
 8 files changed, 563 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
 create mode 100644 drivers/firmware/mediatek/Kconfig
 create mode 100644 drivers/firmware/mediatek/Makefile
 rename drivers/firmware/{ => mediatek}/mtk-adsp-ipc.c (100%)
 create mode 100644 drivers/firmware/mediatek/mtk-hardware-voter.c
 create mode 100644 include/linux/firmware/mediatek/mtk-hardware-voter.h

-- 
2.49.0

[RFC PATCH 1/3] firmware: Move MediaTek ADSP IPC driver to mediatek folder

[AngeloGioacchino Del Regno]

In preparation for adding more MediaTek firmware drivers, create a
new `mediatek` folder and move the mtk-adsp-ipc driver into it.

Also move the Kconfig and Makefile entries to the new ones in the
folder.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/firmware/Kconfig                       | 10 +---------
 drivers/firmware/Makefile                      |  2 +-
 drivers/firmware/mediatek/Kconfig              | 14 ++++++++++++++
 drivers/firmware/mediatek/Makefile             |  3 +++
 drivers/firmware/{ => mediatek}/mtk-adsp-ipc.c |  0
 5 files changed, 19 insertions(+), 10 deletions(-)
 create mode 100644 drivers/firmware/mediatek/Kconfig
 create mode 100644 drivers/firmware/mediatek/Makefile
 rename drivers/firmware/{ => mediatek}/mtk-adsp-ipc.c (100%)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index bbd2155d8483..f35648686f91 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -169,15 +169,6 @@ config INTEL_STRATIX10_RSU
 
 	  Say Y here if you want Intel RSU support.
 
-config MTK_ADSP_IPC
-	tristate "MTK ADSP IPC Protocol driver"
-	depends on MTK_ADSP_MBOX
-	help
-	  Say yes here to add support for the MediaTek ADSP IPC
-	  between host AP (Linux) and the firmware running on ADSP.
-	  ADSP exists on some mtk processors.
-	  Client might use shared memory to exchange information with ADSP.
-
 config SYSFB
 	bool
 	select BOOT_VESA_SUPPORT
@@ -290,6 +281,7 @@ source "drivers/firmware/cirrus/Kconfig"
 source "drivers/firmware/google/Kconfig"
 source "drivers/firmware/efi/Kconfig"
 source "drivers/firmware/imx/Kconfig"
+source "drivers/firmware/mediatek/Kconfig"
 source "drivers/firmware/meson/Kconfig"
 source "drivers/firmware/microchip/Kconfig"
 source "drivers/firmware/psci/Kconfig"
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 4ddec2820c96..5bb382344906 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -13,7 +13,6 @@ obj-$(CONFIG_INTEL_STRATIX10_RSU)     += stratix10-rsu.o
 obj-$(CONFIG_ISCSI_IBFT_FIND)	+= iscsi_ibft_find.o
 obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
 obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
-obj-$(CONFIG_MTK_ADSP_IPC)	+= mtk-adsp-ipc.o
 obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
 obj-$(CONFIG_FW_CFG_SYSFS)	+= qemu_fw_cfg.o
 obj-$(CONFIG_SYSFB)		+= sysfb.o
@@ -27,6 +26,7 @@ obj-y				+= arm_ffa/
 obj-y				+= arm_scmi/
 obj-y				+= broadcom/
 obj-y				+= cirrus/
+obj-y				+= mediatek/
 obj-y				+= meson/
 obj-y				+= microchip/
 obj-$(CONFIG_GOOGLE_FIRMWARE)	+= google/
diff --git a/drivers/firmware/mediatek/Kconfig b/drivers/firmware/mediatek/Kconfig
new file mode 100644
index 000000000000..f6f16e71fbda
--- /dev/null
+++ b/drivers/firmware/mediatek/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+menu "MediaTek firmware drivers"
+
+config MTK_ADSP_IPC
+	tristate "MTK ADSP IPC Protocol driver"
+	depends on MTK_ADSP_MBOX
+	help
+	  Say yes here to add support for the MediaTek ADSP IPC
+	  between host AP (Linux) and the firmware running on ADSP.
+	  ADSP exists on some mtk processors.
+	  Client might use shared memory to exchange information with ADSP.
+
+endmenu
diff --git a/drivers/firmware/mediatek/Makefile b/drivers/firmware/mediatek/Makefile
new file mode 100644
index 000000000000..3c0d9d67d646
--- /dev/null
+++ b/drivers/firmware/mediatek/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_MTK_ADSP_IPC)		+= mtk-adsp-ipc.o
diff --git a/drivers/firmware/mtk-adsp-ipc.c b/drivers/firmware/mediatek/mtk-adsp-ipc.c
similarity index 100%
rename from drivers/firmware/mtk-adsp-ipc.c
rename to drivers/firmware/mediatek/mtk-adsp-ipc.c
-- 
2.49.0

[RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)

[AngeloGioacchino Del Regno]

Add documentation for the new MediaTek Hardware Voter, found in
MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
the MT6991 Dimensity 9400 for Smartphones.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml

diff --git a/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
new file mode 100644
index 000000000000..173b74c23a91
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2025 Collabora Ltd
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Hardware Voter (HWV)
+
+maintainers:
+  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+
+description:
+  The MediaTek Hardware Voter (HWV) is a SoC-internal fixed-function MCU
+  used to collect votes from both the Application Processor and from the
+  various other remote processors present in the SoC, and transparently
+  turn on or off various hardware resources (for example, power domains
+  or system clocks) based on aggregation of votes done in the HWV MCU's
+  internal state machine, therefore guaranteeing synchronization of the
+  hardware resource requests between all components of the SoC and hence
+  avoiding, for example, unclocked or unpowered access to the hardware.
+
+properties:
+  $nodename:
+    pattern: "^system-controller@[0-9a-f]+$"
+
+  compatible:
+    const: mediatek,mt6991-hardware-voter
+
+  reg:
+    items:
+      - description: Address and size of the Hardware Voter MMIO
+
+  power-controller:
+    $ref: /schemas/power/mediatek,power-controller.yaml
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: true
+
+examples:
+ - |
+   scp_hwv: system-controller@14500000 {
+     compatible = "mediatek,mt6991-hardware-voter";
+     reg = <0 0x14500000 0 0x3000>;
+
+     power-controller {
+       compatible = "mediatek,mt8196-hwv-scp-power-controller";
+       #address-cells = <1>;
+       #size-cells = <0>;
+       #power-domain-cells = <1>;
+     };
+   };
+
+   /*
+    * For RFC patch only, this will be removed at patch v1
+    * Note 1: Clock controllers have more than 90 registers in HWV
+    * Note 2: The HWV integrates the power controller, so that's why
+    *         that is a child node of HWV, but the clock controllers
+    *         are completely separated (in hardware) from the HWV's
+    *         physical location (other than address space)... so it
+    *         would be wrong to place those as children of HWV I think.
+    */
+   clock-controller@16640000 {
+     comaptible = "mediatek,mt8196-pericfg-ao";
+     mediatek,hardware-voter = <&scp_hwv>;
+     #clock-cells = <1>;
+   };
-- 
2.49.0

[RFC PATCH 3/3] firmware: mediatek: Add MediaTek Hardware Voter MCU driver

[AngeloGioacchino Del Regno]

Add a driver for the MediaTek Hardware Voter (HWV) MCU, a SoC
internal fixed-function MCU used to collect votes from both
the AP and from the various other remote processors present
in the SoC, guaranteeing synchronization of the HW resource
requests between all components of the SoCs and hence avoiding,
for example, unclocked and/or unpowered access to the hardware.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/firmware/mediatek/Kconfig             |   9 +
 drivers/firmware/mediatek/Makefile            |   1 +
 .../firmware/mediatek/mtk-hardware-voter.c    | 430 ++++++++++++++++++
 .../firmware/mediatek/mtk-hardware-voter.h    |  34 ++
 4 files changed, 474 insertions(+)
 create mode 100644 drivers/firmware/mediatek/mtk-hardware-voter.c
 create mode 100644 include/linux/firmware/mediatek/mtk-hardware-voter.h

diff --git a/drivers/firmware/mediatek/Kconfig b/drivers/firmware/mediatek/Kconfig
index f6f16e71fbda..b1b2d8a82358 100644
--- a/drivers/firmware/mediatek/Kconfig
+++ b/drivers/firmware/mediatek/Kconfig
@@ -11,4 +11,13 @@ config MTK_ADSP_IPC
 	  ADSP exists on some mtk processors.
 	  Client might use shared memory to exchange information with ADSP.
 
+config MTK_HARDWARE_VOTER
+	tristate "MediaTek Hardware Resources Voter"
+	depends on MTK_ADSP_MBOX
+	help
+	  Say yes here to add support for the MediaTek Hardware Voter (HWV),
+	  an interface to the HWV MCU which is used to synchronize requests
+	  for hardware resources between host AP (Linux) and the firmwares
+	  that are running on various integrated remote processors.
+
 endmenu
diff --git a/drivers/firmware/mediatek/Makefile b/drivers/firmware/mediatek/Makefile
index 3c0d9d67d646..06de6f4dc0a3 100644
--- a/drivers/firmware/mediatek/Makefile
+++ b/drivers/firmware/mediatek/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_MTK_ADSP_IPC)		+= mtk-adsp-ipc.o
+obj-$(CONFIG_MTK_HARDWARE_VOTER)	+= mtk-hardware-voter.o
diff --git a/drivers/firmware/mediatek/mtk-hardware-voter.c b/drivers/firmware/mediatek/mtk-hardware-voter.c
new file mode 100644
index 000000000000..5e12beb44a4c
--- /dev/null
+++ b/drivers/firmware/mediatek/mtk-hardware-voter.c
@@ -0,0 +1,430 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek Hardware Voter (HWV) MCU communication driver
+ *
+ * Copyright (C) 2025 Collabora Ltd.
+ *                    AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+ */
+
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/firmware/mediatek/mtk-hardware-voter.h>
+
+#define MTK_HWV_REGISTER_WIDTH_BITS	32
+
+#define MTK_HWV_MTCMOS_POLL_DELAY_US	3
+#define MTK_HWV_MTCMOS_DONE_TIMEOUT_US	(10 * USEC_PER_MSEC)
+#define MTK_HWV_MTCMOS_VOTE_TIMEOUT_US	(3 * USEC_PER_MSEC)
+#define MTK_HWV_CLK_DONE_TIMEOUT_US	30
+
+/**
+ * struct mtk_hardware_voter_pmdomain_regs - Hardware Voter power domain data
+ * @set:        Offset of the HWV SET register
+ * @clear:      Offset of the HWV CLEAR register
+ * @done:       Offset of the HWV DONE register
+ * @enable:     Offset of the HWV ENABLE register
+ * @set_sta:    Offset of the HWV SET STATUS register
+ * @clr_sta:    Offset of the HWV CLEAR STATUS register
+ */
+struct mtk_hardware_voter_pmdomain_regs {
+	u16 set;
+	u16 clear;
+	u16 enable;
+	u16 done;
+	u16 set_sta;
+	u16 clr_sta;
+};
+
+/**
+ * struct mtk_hardware_voter_pmdomain_info - Internal HWV pmdomain info structure
+ * @regmap:            Handle to regmap
+ * @regs:              Set of registers for specific Power Domain HWV instance
+ * @index:             Index of the Power Domain relative to this set of registers
+ *
+ * This structure is used only for driver-internal parameters passing.
+ */
+struct mtk_hardware_voter_pmdomain_info {
+	struct regmap *regmap;
+	const struct mtk_hardware_voter_pmdomain_regs *regs;
+	unsigned int index;
+};
+
+/**
+ * struct mtk_hardware_voter_hw_funcs - HWV Version specific callbacks
+ * @clk_enable:        Callback to execute programming sequence to vote for clock
+ * @pmdomain_enable:   Callback to execute programming sequence to vote for pmdomain
+ */
+struct mtk_hardware_voter_hw_funcs {
+	int (*clk_enable)(struct mtk_hardware_voter *hwv,
+			  const struct mtk_hardware_voter_clk_regs *clk_regs,
+			  unsigned int index, bool enable);
+	int (*pmdomain_enable)(struct device *dev,
+			       struct mtk_hardware_voter_pmdomain_info *hwvpd,
+			       unsigned int index, bool enable);
+};
+
+/**
+ * struct mtk_hardware_voter_pdata - SoC-specific platform data
+ * @funcs:             HWV Version-specific register programming sequence functions
+ * @pmdomain_regs:     HWV Version-specific Power Domain register (offset) sets
+ * @num_pmdomain_regs: Number of Power Domain register sets in this HWV
+ */
+struct mtk_hardware_voter_pdata {
+	const struct mtk_hardware_voter_hw_funcs *funcs;
+	const struct mtk_hardware_voter_pmdomain_regs *pmdomain_regs;
+	unsigned int num_pmdomain_regs;
+};
+
+/**
+ * struct mtk_hardware_voter_priv - Driver private structure
+ * @regmap:            Handle to regmap
+ * @pdata:             SoC-specific platform data
+ */
+struct mtk_hardware_voter_priv {
+	struct regmap *regmap;
+	const struct mtk_hardware_voter_pdata *pdata;
+};
+
+static const struct of_device_id mtk_hardware_voter_match[];
+
+/**
+ * mtk_hardware_voter_get_handle() - Get the MediaTek HWV handle for a device
+ * @dev: Pointer to the device that wants to get a HWV handle
+ *
+ * Return:
+ * Pointer to the mtk_hardware_voter handle for success; or
+ * -EPROBE_DEFER pointer error if the HWV instance is not ready
+ * -ENODEV pointer error if the device pointer is missing
+ * -ENOENT pointer error if the HWV device was not found
+ * -EINVAL pointer error for other errors
+ */
+struct mtk_hardware_voter *mtk_hardware_voter_get_handle(struct device *dev)
+{
+	const struct of_device_id *match_id;
+	struct mtk_hardware_voter *hwv;
+	struct platform_device *pdev;
+	struct device_node *np;
+
+	if (!dev || !dev->parent)
+		return ERR_PTR(-ENODEV);
+
+	np = dev->parent->of_node;
+	if (!np)
+		return ERR_PTR(-EINVAL);
+
+	match_id = of_match_node(mtk_hardware_voter_match, np);
+	if (!match_id)
+		return ERR_PTR(-ENOENT);
+
+	hwv = dev_get_drvdata(&pdev->dev);
+	if (!hwv) {
+		hwv = ERR_PTR(-EPROBE_DEFER);
+		put_device(&pdev->dev);
+		goto end;
+	}
+
+end:
+	of_node_put(np);
+	return hwv;
+}
+
+/**
+ * mtk_hardware_voter_get_by_phandle() - Get the MediaTek HWV handle using DT phandle
+ * @dev: Pointer to the device that wants to get a HWV handle
+ *
+ * The struct device passed to this function must have an associated
+ * devicetree node; this node must have the "mediatek,hardware-voter"
+ * property with a phandle to the Hardware Voter or, if multiple voters
+ * are present, a phandle to the desired Hardware Voter instance.
+ * Return:
+ * Pointer to the mtk_hardware_voter handle for success; or
+ * -EPROBE_DEFER pointer error if the HWV instance is not ready
+ * -ENODEV pointer error if the device pointer is missing
+ * -ENOENT pointer error if the HWV device was not found
+ * -EINVAL pointer error for other errors
+ */
+struct mtk_hardware_voter *mtk_hardware_voter_get_by_phandle(struct device *dev)
+{
+	struct mtk_hardware_voter *hwv;
+	struct platform_device *pdev;
+	struct device_node *np;
+
+	np = of_parse_phandle(dev->of_node, "mediatek,hardware-voter", 0);
+	if (!np)
+		return ERR_PTR(-ENOENT);
+
+	pdev = of_find_device_by_node(np);
+	if (!pdev) {
+		hwv = ERR_PTR(-ENODEV);
+		goto end;
+	}
+
+	hwv = platform_get_drvdata(pdev);
+	if (!hwv) {
+		hwv = ERR_PTR(-EPROBE_DEFER);
+		put_device(&pdev->dev);
+		goto end;
+	}
+
+end:
+	of_node_put(np);
+	return hwv;
+}
+
+/**
+ * mtk_hardware_voter_pmdomain_enable() - Vote power domain enable/disable
+ * @hwv:    Handle to the Hardware Voter instance
+ * @index:  Index of the power domain in the Hardware Voter
+ * @enable: Vote to enable or disable a power domain
+ *
+ * The programming sequence varies with Hardware Voter MCU versions;
+ * this function is responsible for executing a callback to send the
+ * HW Voter version specific register programming sequence to vote for
+ * enabling or disabling a power domain.
+ */
+int mtk_hardware_voter_pmdomain_enable(struct mtk_hardware_voter *hwv,
+				       unsigned int index, bool enable)
+{
+	struct mtk_hardware_voter_pmdomain_info hwv_pmdomain_info;
+	const struct mtk_hardware_voter_pdata *pdata = hwv->priv->pdata;
+	const struct mtk_hardware_voter_pmdomain_regs *regs = pdata->pmdomain_regs;
+	unsigned int register_set_num = index % MTK_HWV_REGISTER_WIDTH_BITS;
+	unsigned int register_set_idx = index / MTK_HWV_REGISTER_WIDTH_BITS;
+
+	if (register_set_num > pdata->num_pmdomain_regs)
+		return -ENXIO;
+
+	if (register_set_idx > (MTK_HWV_REGISTER_WIDTH_BITS - 1))
+		return -EINVAL;
+
+	hwv_pmdomain_info.regmap = hwv->priv->regmap;
+	hwv_pmdomain_info.index = register_set_idx;
+	hwv_pmdomain_info.regs = &regs[register_set_num];
+
+	return pdata->funcs->pmdomain_enable(hwv->dev, &hwv_pmdomain_info, index, enable);
+}
+EXPORT_SYMBOL_GPL(mtk_hardware_voter_pmdomain_enable);
+
+/**
+ * mtk_hardware_voter_clk_enable() - Vote clock gating or ungating
+ * @hwv:    Handle to the Hardware Voter instance
+ * @regs:   Registers for the specific clock domain's Hardware Voter
+ * @index:  Index of the power domain in the Hardware Voter
+ * @enable: Vote to enable or disable a power domain
+ *
+ * The programming sequence varies with Hardware Voter MCU versions;
+ * this function is responsible for executing a callback to send the
+ * HW Voter version specific register programming sequence to vote for
+ * gating or ungating a clock.
+ */
+int mtk_hardware_voter_clk_enable(struct mtk_hardware_voter *hwv,
+				  const struct mtk_hardware_voter_clk_regs *regs,
+				  unsigned int index, bool enable)
+{
+	const struct mtk_hardware_voter_pdata *pdata = hwv->priv->pdata;
+
+	return pdata->funcs->clk_enable(hwv, regs, index, enable);
+}
+EXPORT_SYMBOL_GPL(mtk_hardware_voter_clk_enable);
+
+static bool mtk_hardware_voter_v1_pmdomain_is_disable_done(
+		struct mtk_hardware_voter_pmdomain_info *hwvpd)
+{
+	u32 regs[2] = { hwvpd->regs->done, hwvpd->regs->clr_sta };
+	u32 val[2];
+	u32 mask = BIT(hwvpd->index);
+
+	regmap_multi_reg_read(hwvpd->regmap, regs, val, 2);
+
+	/* Disable is done when the bit is set in DONE, cleared in CLR_STA */
+	return (val[0] & mask) && !(val[1] & mask);
+}
+
+static bool mtk_hardware_voter_v1_pmdomain_is_enable_done(
+		struct mtk_hardware_voter_pmdomain_info *hwvpd)
+{
+	u32 regs[3] = { hwvpd->regs->done, hwvpd->regs->enable, hwvpd->regs->set_sta };
+	u32 val[3];
+	u32 mask = BIT(hwvpd->index);
+
+	regmap_multi_reg_read(hwvpd->regmap, regs, val, 3);
+
+	/* Enable is done when the bit is set in DONE and EN, cleared in SET_STA */
+	return (val[0] & mask) && (val[1] & mask) && !(val[2] & mask);
+}
+
+static int mtk_hardware_voter_v1_pmdomain_enable(struct device *dev,
+						 struct mtk_hardware_voter_pmdomain_info *hwvpd,
+						 unsigned int index, bool enable)
+{
+	const struct mtk_hardware_voter_pmdomain_regs *regs = hwvpd->regs;
+	struct regmap *regmap = hwvpd->regmap;
+	u32 val;
+	int ret;
+
+	/* Make sure the HW Voter is idle and able to accept commands */
+	ret = regmap_read_poll_timeout_atomic(regmap, regs->done, val,
+					      val & BIT(index),
+					      MTK_HWV_MTCMOS_POLL_DELAY_US,
+					      MTK_HWV_MTCMOS_DONE_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Failed to power on: HW Voter busy.\n");
+		return ret;
+	}
+
+	/*
+	 * Instructing the HWV to power on/off the MTCMOS (power domains):
+	 * For power on, the same bit will be cleared from the SET register
+	 * by the hardware immediately after setting it;
+	 * For power off, the bit will be kept set in the CLEAR register
+	 */
+	regmap_write(regmap, enable ? regs->set : regs->clear, BIT(index));
+
+	if (enable) {
+		/*
+		 * Wait until the HWV sets the bit again, signalling that its
+		 * internal state machine was started and it now processing
+		 * the vote command.
+		 */
+		ret = regmap_read_poll_timeout_atomic(regmap, regs->set,
+						      val, val & BIT(index),
+						      0, MTK_HWV_MTCMOS_VOTE_TIMEOUT_US);
+		if (ret) {
+			dev_err(dev, "Failed to power on: HW Voter not starting.\n");
+			return ret;
+		}
+
+		/* Wait for ACK, signalling that the MTCMOS was enabled */
+		ret = readx_poll_timeout_atomic(mtk_hardware_voter_v1_pmdomain_is_enable_done,
+						hwvpd, val, val,
+						MTK_HWV_MTCMOS_POLL_DELAY_US,
+						MTK_HWV_MTCMOS_DONE_TIMEOUT_US);
+		if (ret) {
+			dev_err(dev, "Failed to power on: HW Voter ACK timeout.\n");
+			return ret;
+		}
+	} else {
+		/*
+		 * Wait until the HWV clears the bit, signalling that its
+		 * internal state machine was started and it now processing
+		 * the clear command.
+		 */
+		ret = regmap_read_poll_timeout_atomic(regmap, regs->clear,
+						      val, !(val & BIT(index)),
+						      0, MTK_HWV_MTCMOS_VOTE_TIMEOUT_US);
+		if (ret) {
+			dev_err(dev, "Failed to power off: HW Voter not starting.\n");
+			return ret;
+		}
+
+		/* Poweroff needs 100us for the HW to stabilize */
+		udelay(100);
+
+		/* Wait for ACK, signalling that the MTCMOS was disabled */
+		ret = readx_poll_timeout_atomic(mtk_hardware_voter_v1_pmdomain_is_disable_done,
+						hwvpd, val, val,
+						MTK_HWV_MTCMOS_POLL_DELAY_US,
+						MTK_HWV_MTCMOS_DONE_TIMEOUT_US);
+		if (ret) {
+			dev_err(dev, "Failed to power on: HW Voter ACK timeout.\n");
+			return ret;
+		}
+	};
+
+	return 0;
+}
+
+static int mtk_hardware_voter_v1_clk_enable(struct mtk_hardware_voter *hwv,
+					    const struct mtk_hardware_voter_clk_regs *regs,
+					    unsigned int index, bool enable)
+{
+	struct regmap *regmap = hwv->priv->regmap;
+	u32 val;
+	int ret;
+
+	ret = regmap_write(regmap, enable ? regs->set : regs->clear, BIT(index));
+	if (ret)
+		return ret;
+
+	return regmap_read_poll_timeout_atomic(regmap, regs->status, val,
+					       val & BIT(index),
+					       0, MTK_HWV_CLK_DONE_TIMEOUT_US);
+}
+
+static const struct regmap_config mtk_hardware_voter_regmap_config = {
+	.reg_bits = MTK_HWV_REGISTER_WIDTH_BITS,
+	.val_bits = MTK_HWV_REGISTER_WIDTH_BITS,
+	.reg_stride = 4,
+	.max_register = 0x3000,
+};
+
+static int mtk_hardware_voter_probe(struct platform_device *pdev)
+{
+	struct mtk_hardware_voter_priv *priv;
+	struct mtk_hardware_voter *hwv;
+	struct regmap *regmap;
+	void __iomem *base;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio(&pdev->dev, base,
+				       &mtk_hardware_voter_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(&pdev->dev, PTR_ERR(regmap),
+				     "Cannot initialize regmap.\n");
+
+	hwv = devm_kzalloc(&pdev->dev, sizeof(*hwv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	hwv->dev = &pdev->dev;
+	hwv->priv = priv;
+
+	priv->pdata = of_device_get_match_data(&pdev->dev);
+	priv->regmap = regmap;
+	platform_set_drvdata(pdev, &hwv);
+
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static const struct mtk_hardware_voter_hw_funcs mtk_hwv_v1_funcs = {
+	.clk_enable = mtk_hardware_voter_v1_clk_enable,
+	.pmdomain_enable = mtk_hardware_voter_v1_pmdomain_enable,
+};
+
+static const struct mtk_hardware_voter_pmdomain_regs mtk_hwv_v1_pmdomain_regs[] = {
+	/* SET    CLR     EN     DONE   SETSTA  CLRSTA */
+	{ 0x218, 0x21c, 0x1410, 0x141c, 0x146c, 0x1470 },
+	{ 0x220, 0x224, 0x1420, 0x142c, 0x1474, 0x1478 }
+};
+
+static const struct mtk_hardware_voter_pdata mt6991_hwv_pdata = {
+	.funcs = &mtk_hwv_v1_funcs,
+	.pmdomain_regs = mtk_hwv_v1_pmdomain_regs,
+	.num_pmdomain_regs = ARRAY_SIZE(mtk_hwv_v1_pmdomain_regs),
+};
+
+static const struct of_device_id mtk_hardware_voter_match[] = {
+	{ .compatible = "mediatek,mt6991-hardware-voter", .data = &mt6991_hwv_pdata },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver mtk_hardware_voter_driver = {
+	.driver = {
+		.name = "mtk-hardware-voter",
+		.of_match_table = mtk_hardware_voter_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = mtk_hardware_voter_probe,
+};
+builtin_platform_driver(mtk_hardware_voter_driver);
diff --git a/include/linux/firmware/mediatek/mtk-hardware-voter.h b/include/linux/firmware/mediatek/mtk-hardware-voter.h
new file mode 100644
index 000000000000..585a12db393c
--- /dev/null
+++ b/include/linux/firmware/mediatek/mtk-hardware-voter.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025 Collabora Ltd
+ *                    AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+ */
+
+#ifndef MTK_HARDWARE_VOTER_H
+#define MTK_HARDWARE_VOTER_H
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+struct mtk_hardware_voter_clk_regs {
+	u16 set;
+	u16 clear;
+	u16 status;
+};
+
+struct mtk_hardware_voter_priv;
+
+struct mtk_hardware_voter {
+	struct device *dev;
+	struct mtk_hardware_voter_priv *priv;
+};
+
+struct mtk_hardware_voter *mtk_hardware_voter_get_handle(struct device *dev);
+struct mtk_hardware_voter *mtk_hardware_voter_get_by_phandle(struct device *dev);
+int mtk_hardware_voter_clk_enable(struct mtk_hardware_voter *hwv,
+				  const struct mtk_hardware_voter_clk_regs *clk_regs,
+				  unsigned int index, bool enable);
+int mtk_hardware_voter_pmdomain_enable(struct mtk_hardware_voter *hwv,
+				       unsigned int index, bool enable);
+
+#endif /* MTK_HARDWARE_VOTER_H */
-- 
2.49.0

Re: [RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)

[Rob Herring (Arm)]

On Tue, 01 Jul 2025 17:11:48 +0200, AngeloGioacchino Del Regno wrote:
> Add documentation for the new MediaTek Hardware Voter, found in
> MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
> the MT6991 Dimensity 9400 for Smartphones.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
> 

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

yamllint warnings/errors:
./Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml:44:2: [warning] wrong indentation: expected 2 but found 1 (indentation)

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.example.dts:39.35-43.11: Warning (unit_address_vs_reg): /example-0/clock-controller@16640000: node has a unit name, but no reg or ranges property
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.example.dtb: system-controller@14500000 (mediatek,mt6991-hardware-voter): power-controller:compatible:0: 'mediatek,mt8196-hwv-scp-power-controller' is not one of ['mediatek,mt6735-power-controller', 'mediatek,mt6795-power-controller', 'mediatek,mt6893-power-controller', 'mediatek,mt8167-power-controller', 'mediatek,mt8173-power-controller', 'mediatek,mt8183-power-controller', 'mediatek,mt8186-power-controller', 'mediatek,mt8188-power-controller', 'mediatek,mt8192-power-controller', 'mediatek,mt8195-power-controller', 'mediatek,mt8365-power-controller']
	from schema $id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.example.dtb: system-controller@14500000 (mediatek,mt6991-hardware-voter): reg: [[0, 340787200], [0, 12288]] is too long
	from schema $id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.example.dtb: /example-0/system-controller@14500000/power-controller: failed to match any schema with compatible: ['mediatek,mt8196-hwv-scp-power-controller']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250701151149.136365-3-angelogioacchino.delregno@collabora.com

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

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

pip3 install dtschema --upgrade

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

Re: [RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)

[Krzysztof Kozlowski]

On Tue, Jul 01, 2025 at 05:11:48PM +0200, AngeloGioacchino Del Regno wrote:
> Add documentation for the new MediaTek Hardware Voter, found in
> MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
> the MT6991 Dimensity 9400 for Smartphones.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
> 
> diff --git a/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
> new file mode 100644
> index 000000000000..173b74c23a91
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2025 Collabora Ltd
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Hardware Voter (HWV)
> +
> +maintainers:
> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> +
> +description:
> +  The MediaTek Hardware Voter (HWV) is a SoC-internal fixed-function MCU
> +  used to collect votes from both the Application Processor and from the
> +  various other remote processors present in the SoC, and transparently
> +  turn on or off various hardware resources (for example, power domains
> +  or system clocks) based on aggregation of votes done in the HWV MCU's
> +  internal state machine, therefore guaranteeing synchronization of the
> +  hardware resource requests between all components of the SoC and hence
> +  avoiding, for example, unclocked or unpowered access to the hardware.
> +
> +properties:
> +  $nodename:
> +    pattern: "^system-controller@[0-9a-f]+$"
> +
> +  compatible:
> +    const: mediatek,mt6991-hardware-voter
> +
> +  reg:
> +    items:
> +      - description: Address and size of the Hardware Voter MMIO
> +

No resources here, so this should go to power controller

> +  power-controller:
> +    $ref: /schemas/power/mediatek,power-controller.yaml
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: true
> +
> +examples:
> + - |
> +   scp_hwv: system-controller@14500000 {
> +     compatible = "mediatek,mt6991-hardware-voter";
> +     reg = <0 0x14500000 0 0x3000>;
> +
> +     power-controller {
> +       compatible = "mediatek,mt8196-hwv-scp-power-controller";

mt8196 in mt6991 is very confusing.

Anyway, this does not address my comment at all. You again create some
sort of syscon for voting, so no. You are supposed to use generic API
for voting: clocks, power domains, interconnects - whatever is there
applicable or necessary.

Best regards,
Krzysztof

Re: [RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)

[AngeloGioacchino Del Regno]

Il 02/07/25 08:50, Krzysztof Kozlowski ha scritto:
> On Tue, Jul 01, 2025 at 05:11:48PM +0200, AngeloGioacchino Del Regno wrote:
>> Add documentation for the new MediaTek Hardware Voter, found in
>> MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
>> the MT6991 Dimensity 9400 for Smartphones.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
>>   1 file changed, 70 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
>> new file mode 100644
>> index 000000000000..173b74c23a91
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware-voter.yaml
>> @@ -0,0 +1,70 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +# Copyright 2025 Collabora Ltd
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MediaTek Hardware Voter (HWV)
>> +
>> +maintainers:
>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> +
>> +description:
>> +  The MediaTek Hardware Voter (HWV) is a SoC-internal fixed-function MCU
>> +  used to collect votes from both the Application Processor and from the
>> +  various other remote processors present in the SoC, and transparently
>> +  turn on or off various hardware resources (for example, power domains
>> +  or system clocks) based on aggregation of votes done in the HWV MCU's
>> +  internal state machine, therefore guaranteeing synchronization of the
>> +  hardware resource requests between all components of the SoC and hence
>> +  avoiding, for example, unclocked or unpowered access to the hardware.
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^system-controller@[0-9a-f]+$"
>> +
>> +  compatible:
>> +    const: mediatek,mt6991-hardware-voter
>> +
>> +  reg:
>> +    items:
>> +      - description: Address and size of the Hardware Voter MMIO
>> +
> 
> No resources here, so this should go to power controller
> 
>> +  power-controller:
>> +    $ref: /schemas/power/mediatek,power-controller.yaml
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: true
>> +
>> +examples:
>> + - |
>> +   scp_hwv: system-controller@14500000 {
>> +     compatible = "mediatek,mt6991-hardware-voter";
>> +     reg = <0 0x14500000 0 0x3000>;
>> +
>> +     power-controller {
>> +       compatible = "mediatek,mt8196-hwv-scp-power-controller";
> 
> mt8196 in mt6991 is very confusing.
> 

Yeah that wasn't intentional; fyi, it's almost the same soc, that's why I mixed
them up... :-)

> Anyway, this does not address my comment at all. You again create some
> sort of syscon for voting, so no. You are supposed to use generic API
> for voting: clocks, power domains, interconnects - whatever is there
> applicable or necessary.
> 

Making that loud and clear: Interconnect is not applicable.

The only way to do what you're proposing would be to add a bunch of `reg`
to each devicetree node for each clock controller and each power controller;
I can do that, but looks a bit dirty - and still yet another syscon-like
alternative, but without having a real syscon declared in there.

Mind you - both clock and power controllers are writing both to their own
register space (and enabling external regulators, etc, for power domains)
and to the hardware voter MMIO (which means that the HWV, in hardware, is
fundamentally broken).

After this reply, the only option that is left to me is the following:

		topckgen: clock-controller@10000000 {
			compatible = "mediatek,mt8196-topckgen", "syscon";
			reg = <0 0x10000000 0 0x800>, <0 0x14500010 0 0x48>,
			      <0 0x14502c08 0 0x24>;
			reg-names = "base", "hwvoter-base", "hwvoter-status";
			#clock-cells = <1>;
		};

		imp_iic_wrap_north: clock-controller@13c30000 {
			compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
			reg = <0 0x13c30000 0 0x1000>, <0 0x14500000 0 0xc>,
			      <0 0x14502c00 0 0xc>;
			reg-names = "base", "hwvoter-base", "hwvoter-status";
			#clock-cells = <1>;
		};

		/* Power Manager with Hardware Voter */
		spm_hwv: power-controller@14500218 {
			compatible = "mediatek,mt8196-hwv-scp-power-controller";
			reg = <0 0x14500218 0 0x20>, <0 0x14501410 0 0x20>,
			      <0 0x14505514 0 0xc>;
			reg-names = "hwvoter-base", "hwvoter-status", "hwvoter-ack";
			#address-cells = <1>;
			#size-cells = <0>;
			#power-domain-cells = <1>;

			/* SCPSYS hardware voter power domains */
			mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
				..... etc, all power domains

At this point, I'm really not sure that this would be better than just passing
the mediatek,hardware-voter syscon to the clock controllers - as what I've done
previously was effectively representing the hardware in the devicetree as it is,
matching the real HW layout 1:1 (because again, each of the whole HWV MCU(s) are
embedded into each of the two power controllers, one for System power, and one
for Multimedia power).

(btw, hardware speaking, the power controller is child of a system controller:
there are two system controllers - "scpsystem" is for "compute part", and the
"hfrpsystem" is for the "multimedia part" of the soc).

  _______________________________________
|                                       |
| SYSTEM CONTROLLER (SCPSYS or HFRPSYS) |
|   _____________________               |
|  |                     |              | <===> Clock Controllers (more than one)
|  | Power Controller    |     SOME     |       (provide subsystem clocks for iso
|  |                     |    OTHER     |        during power domain enablement
|  |     ______________  |   BLOCKS     |        even if a PD is voted)
|  |    |              | |              |       non-subsystem clocks are voted,
|  |    | HW Voter MCU | |              |       but subsystem ones are not voted
|  |    |______________| |              |
|  |_____________________|              | ===> Rest of the SoC
|_______________________________________|


Hence I'm asking you - does your idea still stand?

Because after this, sorry for that - this doesn't want to be an attack - but
I'm starting to have doubts about an approach that doesn't involve syscons.

Cheers,
Angelo

Re: [RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)

[AngeloGioacchino Del Regno]

Il 03/07/25 10:56, AngeloGioacchino Del Regno ha scritto:
> Il 02/07/25 08:50, Krzysztof Kozlowski ha scritto:
>> On Tue, Jul 01, 2025 at 05:11:48PM +0200, AngeloGioacchino Del Regno wrote:
>>> Add documentation for the new MediaTek Hardware Voter, found in
>>> MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
>>> the MT6991 Dimensity 9400 for Smartphones.
>>>
>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>> ---
>>>   .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
>>>   1 file changed, 70 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991- 
>>> hardware-voter.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/mediatek,mt6991- 
>>> hardware-voter.yaml b/Documentation/devicetree/bindings/firmware/ 
>>> mediatek,mt6991-hardware-voter.yaml
>>> new file mode 100644
>>> index 000000000000..173b74c23a91
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware- 
>>> voter.yaml
>>> @@ -0,0 +1,70 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +# Copyright 2025 Collabora Ltd
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek Hardware Voter (HWV)
>>> +
>>> +maintainers:
>>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>> +
>>> +description:
>>> +  The MediaTek Hardware Voter (HWV) is a SoC-internal fixed-function MCU
>>> +  used to collect votes from both the Application Processor and from the
>>> +  various other remote processors present in the SoC, and transparently
>>> +  turn on or off various hardware resources (for example, power domains
>>> +  or system clocks) based on aggregation of votes done in the HWV MCU's
>>> +  internal state machine, therefore guaranteeing synchronization of the
>>> +  hardware resource requests between all components of the SoC and hence
>>> +  avoiding, for example, unclocked or unpowered access to the hardware.
>>> +
>>> +properties:
>>> +  $nodename:
>>> +    pattern: "^system-controller@[0-9a-f]+$"
>>> +
>>> +  compatible:
>>> +    const: mediatek,mt6991-hardware-voter
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: Address and size of the Hardware Voter MMIO
>>> +
>>
>> No resources here, so this should go to power controller
>>
>>> +  power-controller:
>>> +    $ref: /schemas/power/mediatek,power-controller.yaml
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: true
>>> +
>>> +examples:
>>> + - |
>>> +   scp_hwv: system-controller@14500000 {
>>> +     compatible = "mediatek,mt6991-hardware-voter";
>>> +     reg = <0 0x14500000 0 0x3000>;
>>> +
>>> +     power-controller {
>>> +       compatible = "mediatek,mt8196-hwv-scp-power-controller";
>>
>> mt8196 in mt6991 is very confusing.
>>
> 
> Yeah that wasn't intentional; fyi, it's almost the same soc, that's why I mixed
> them up... :-)
> 
>> Anyway, this does not address my comment at all. You again create some
>> sort of syscon for voting, so no. You are supposed to use generic API
>> for voting: clocks, power domains, interconnects - whatever is there
>> applicable or necessary.
>>
> 
> Making that loud and clear: Interconnect is not applicable.
> 
> The only way to do what you're proposing would be to add a bunch of `reg`
> to each devicetree node for each clock controller and each power controller;
> I can do that, but looks a bit dirty - and still yet another syscon-like
> alternative, but without having a real syscon declared in there.
> 
> Mind you - both clock and power controllers are writing both to their own
> register space (and enabling external regulators, etc, for power domains)
> and to the hardware voter MMIO (which means that the HWV, in hardware, is
> fundamentally broken).
> 
> After this reply, the only option that is left to me is the following:
> 
>          topckgen: clock-controller@10000000 {
>              compatible = "mediatek,mt8196-topckgen", "syscon";
>              reg = <0 0x10000000 0 0x800>, <0 0x14500010 0 0x48>,
>                    <0 0x14502c08 0 0x24>;
>              reg-names = "base", "hwvoter-base", "hwvoter-status";
>              #clock-cells = <1>;
>          };
> 
>          imp_iic_wrap_north: clock-controller@13c30000 {
>              compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
>              reg = <0 0x13c30000 0 0x1000>, <0 0x14500000 0 0xc>,
>                    <0 0x14502c00 0 0xc>;
>              reg-names = "base", "hwvoter-base", "hwvoter-status";
>              #clock-cells = <1>;
>          };
> 
>          /* Power Manager with Hardware Voter */
>          spm_hwv: power-controller@14500218 {
>              compatible = "mediatek,mt8196-hwv-scp-power-controller";
>              reg = <0 0x14500218 0 0x20>, <0 0x14501410 0 0x20>,
>                    <0 0x14505514 0 0xc>;
>              reg-names = "hwvoter-base", "hwvoter-status", "hwvoter-ack";
>              #address-cells = <1>;
>              #size-cells = <0>;
>              #power-domain-cells = <1>;
> 
>              /* SCPSYS hardware voter power domains */
>              mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
>                  ..... etc, all power domains
> 
> At this point, I'm really not sure that this would be better than just passing
> the mediatek,hardware-voter syscon to the clock controllers - as what I've done
> previously was effectively representing the hardware in the devicetree as it is,
> matching the real HW layout 1:1 (because again, each of the whole HWV MCU(s) are
> embedded into each of the two power controllers, one for System power, and one
> for Multimedia power).
> 
> (btw, hardware speaking, the power controller is child of a system controller:
> there are two system controllers - "scpsystem" is for "compute part", and the
> "hfrpsystem" is for the "multimedia part" of the soc).
> 
>   _______________________________________
> |                                       |
> | SYSTEM CONTROLLER (SCPSYS or HFRPSYS) |
> |   _____________________               |
> |  |                     |              | <===> Clock Controllers (more than one)
> |  | Power Controller    |     SOME     |       (provide subsystem clocks for iso
> |  |                     |    OTHER     |        during power domain enablement
> |  |     ______________  |   BLOCKS     |        even if a PD is voted)
> |  |    |              | |              |       non-subsystem clocks are voted,
> |  |    | HW Voter MCU | |              |       but subsystem ones are not voted
> |  |    |______________| |              |
> |  |_____________________|              | ===> Rest of the SoC
> |_______________________________________|
> 
> 
> Hence I'm asking you - does your idea still stand?
> 
> Because after this, sorry for that - this doesn't want to be an attack - but
> I'm starting to have doubts about an approach that doesn't involve syscons.
> 
> Cheers,
> Angelo

Sorry for the double reply, wanted to add some more words :-)

As a note, I also thought about doing the following:

	/* Secondary SCPSYS block with HWV capabilities */
	scp1_hwv: system-controller@14500000 {
		compatible = "mediatek,mt8196-scpsys", "syscon", "simple-mfd";
		reg = <0 0x14500000 0 0x3000>;

		/* SCP Power Manager with Hardware Voter */
		spm_hwv: power-controller {
			compatible = "mediatek,mt8196-hwv-scp-power-controller";
			#address-cells = <1>;
			#size-cells = <0>;
			#power-domain-cells = <1>;

			/* SCPSYS hardware voter power domains */
			mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
			..... etc etc
			};
		};

		imp_iic_wrap_north: clock-controller@13c30000 {
			compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
			reg = <0 0x13c30000 0 0x1000>;
			#clock-cells = <1>;
		};
	};

...but that's also not applicable, because the clock controllers are physically
*not* inside of the scpsys1 block, so that would *also* misrepresent the hardware
in the devicetree (besides still using a syscon in a way or another).

So... I really don't see any way out of that, which really leaves me with the two
options that I described in the previous reply.

Summarizing, either:
  - Adding hwv MMIOs (a bunch of, and each very small) to each clock controller (but
    still all of them are poking at the same HWV controller, and I foresee that this
    will backfire in some future iteration of the HWV hardware)
  - Reverting back to using the "mediatek,hardware-voter" syscon, like done in
    https://lore.kernel.org/20250624143220.244549-10-laura.nao@collabora.com

I tried really hard and thought about this for weeks (actually, started even before
your feedback on Laura's series), but now I'm out of practical options that are
both correctly representing the hardware and not making the implementation fragile
(or actually more fragile than the actually broken HW implementation's fragility,
anyway).

And besides - re-reading what I wrote after a bunch of days, the first option of
adding a bunch of hwv mmios to all of the clock controllers is, in my opinion, a
(dirty) hack - because those mmios don't belong to the clock controllers, and would
again misrepresent the hardware in DT - especially keeping in mind the fact that
the clock controllers can be controlled with *and* (not or) without the HWV (and in
some instances, even if using HWV, we must still write to the clock controllers'
mmio for extra programming, as explained before).

Every second I think about this I get more and more convinced that my way of
passing the SCPSYS-HWV system controller handle as a syscon is right.

Angelo

Re: [RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)

[Laura Nao]

On 7/7/25 12:40, AngeloGioacchino Del Regno wrote:
> Il 03/07/25 10:56, AngeloGioacchino Del Regno ha scritto:
>> Il 02/07/25 08:50, Krzysztof Kozlowski ha scritto:
>>> On Tue, Jul 01, 2025 at 05:11:48PM +0200, AngeloGioacchino Del Regno wrote:
>>>> Add documentation for the new MediaTek Hardware Voter, found in
>>>> MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
>>>> the MT6991 Dimensity 9400 for Smartphones.
>>>>
>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>> ---
>>>>   .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
>>>>   1 file changed, 70 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991- hardware-voter.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/firmware/mediatek,mt6991- hardware-voter.yaml b/Documentation/devicetree/bindings/firmware/ mediatek,mt6991-hardware-voter.yaml
>>>> new file mode 100644
>>>> index 000000000000..173b74c23a91
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware- voter.yaml
>>>> @@ -0,0 +1,70 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +# Copyright 2025 Collabora Ltd
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: MediaTek Hardware Voter (HWV)
>>>> +
>>>> +maintainers:
>>>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>> +
>>>> +description:
>>>> +  The MediaTek Hardware Voter (HWV) is a SoC-internal fixed-function MCU
>>>> +  used to collect votes from both the Application Processor and from the
>>>> +  various other remote processors present in the SoC, and transparently
>>>> +  turn on or off various hardware resources (for example, power domains
>>>> +  or system clocks) based on aggregation of votes done in the HWV MCU's
>>>> +  internal state machine, therefore guaranteeing synchronization of the
>>>> +  hardware resource requests between all components of the SoC and hence
>>>> +  avoiding, for example, unclocked or unpowered access to the hardware.
>>>> +
>>>> +properties:
>>>> +  $nodename:
>>>> +    pattern: "^system-controller@[0-9a-f]+$"
>>>> +
>>>> +  compatible:
>>>> +    const: mediatek,mt6991-hardware-voter
>>>> +
>>>> +  reg:
>>>> +    items:
>>>> +      - description: Address and size of the Hardware Voter MMIO
>>>> + 
>>>
>>> No resources here, so this should go to power controller
>>>
>>>> +  power-controller:
>>>> +    $ref: /schemas/power/mediatek,power-controller.yaml
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +
>>>> +additionalProperties: true
>>>> +
>>>> +examples:
>>>> + - |
>>>> +   scp_hwv: system-controller@14500000 {
>>>> +     compatible = "mediatek,mt6991-hardware-voter";
>>>> +     reg = <0 0x14500000 0 0x3000>;
>>>> +
>>>> +     power-controller {
>>>> +       compatible = "mediatek,mt8196-hwv-scp-power-controller"; 
>>>
>>> mt8196 in mt6991 is very confusing.
>>>
>>
>> Yeah that wasn't intentional; fyi, it's almost the same soc, that's why I mixed
>> them up... :-)
>>
>>> Anyway, this does not address my comment at all. You again create some
>>> sort of syscon for voting, so no. You are supposed to use generic API
>>> for voting: clocks, power domains, interconnects - whatever is there
>>> applicable or necessary.
>>>
>>
>> Making that loud and clear: Interconnect is not applicable.
>>
>> The only way to do what you're proposing would be to add a bunch of `reg`
>> to each devicetree node for each clock controller and each power controller;
>> I can do that, but looks a bit dirty - and still yet another syscon-like
>> alternative, but without having a real syscon declared in there.
>>
>> Mind you - both clock and power controllers are writing both to their own
>> register space (and enabling external regulators, etc, for power domains)
>> and to the hardware voter MMIO (which means that the HWV, in hardware, is
>> fundamentally broken).
>>
>> After this reply, the only option that is left to me is the following:
>>
>>          topckgen: clock-controller@10000000 {
>>              compatible = "mediatek,mt8196-topckgen", "syscon";
>>              reg = <0 0x10000000 0 0x800>, <0 0x14500010 0 0x48>,
>>                    <0 0x14502c08 0 0x24>;
>>              reg-names = "base", "hwvoter-base", "hwvoter-status";
>>              #clock-cells = <1>;
>>          };
>>
>>          imp_iic_wrap_north: clock-controller@13c30000 {
>>              compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
>>              reg = <0 0x13c30000 0 0x1000>, <0 0x14500000 0 0xc>,
>>                    <0 0x14502c00 0 0xc>;
>>              reg-names = "base", "hwvoter-base", "hwvoter-status";
>>              #clock-cells = <1>;
>>          };
>>
>>          /* Power Manager with Hardware Voter */
>>          spm_hwv: power-controller@14500218 {
>>              compatible = "mediatek,mt8196-hwv-scp-power-controller";
>>              reg = <0 0x14500218 0 0x20>, <0 0x14501410 0 0x20>,
>>                    <0 0x14505514 0 0xc>;
>>              reg-names = "hwvoter-base", "hwvoter-status", "hwvoter-ack";
>>              #address-cells = <1>;
>>              #size-cells = <0>;
>>              #power-domain-cells = <1>;
>>
>>              /* SCPSYS hardware voter power domains */
>>              mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
>>                  ..... etc, all power domains
>>
>> At this point, I'm really not sure that this would be better than just passing
>> the mediatek,hardware-voter syscon to the clock controllers - as what I've done
>> previously was effectively representing the hardware in the devicetree as it is,
>> matching the real HW layout 1:1 (because again, each of the whole HWV MCU(s) are
>> embedded into each of the two power controllers, one for System power, and one
>> for Multimedia power).
>>
>> (btw, hardware speaking, the power controller is child of a system controller:
>> there are two system controllers - "scpsystem" is for "compute part", and the
>> "hfrpsystem" is for the "multimedia part" of the soc).
>>
>>   _______________________________________
>> |                                       |
>> | SYSTEM CONTROLLER (SCPSYS or HFRPSYS) |
>> |   _____________________               |
>> |  |                     |              | <===> Clock Controllers (more than one)
>> |  | Power Controller    |     SOME     |       (provide subsystem clocks for iso
>> |  |                     |    OTHER     |        during power domain enablement
>> |  |     ______________  |   BLOCKS     |        even if a PD is voted)
>> |  |    |              | |              |       non-subsystem clocks are voted,
>> |  |    | HW Voter MCU | |              |       but subsystem ones are not voted
>> |  |    |______________| |              |
>> |  |_____________________|              | ===> Rest of the SoC
>> |_______________________________________|
>>
>>
>> Hence I'm asking you - does your idea still stand?
>>
>> Because after this, sorry for that - this doesn't want to be an attack - but
>> I'm starting to have doubts about an approach that doesn't involve syscons.
>>
>> Cheers,
>> Angelo 
>
> Sorry for the double reply, wanted to add some more words :-)
>
> As a note, I also thought about doing the following:
>
>     /* Secondary SCPSYS block with HWV capabilities */
>     scp1_hwv: system-controller@14500000 {
>         compatible = "mediatek,mt8196-scpsys", "syscon", "simple-mfd";
>         reg = <0 0x14500000 0 0x3000>;
>
>         /* SCP Power Manager with Hardware Voter */
>         spm_hwv: power-controller {
>             compatible = "mediatek,mt8196-hwv-scp-power-controller";
>             #address-cells = <1>;
>             #size-cells = <0>;
>             #power-domain-cells = <1>;
>
>             /* SCPSYS hardware voter power domains */
>             mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
>             ..... etc etc
>             };
>         };
>
>         imp_iic_wrap_north: clock-controller@13c30000 {
>             compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
>             reg = <0 0x13c30000 0 0x1000>;
>             #clock-cells = <1>;
>         };
>     };
>
> ...but that's also not applicable, because the clock controllers are physically
> *not* inside of the scpsys1 block, so that would *also* misrepresent the hardware
> in the devicetree (besides still using a syscon in a way or another).
>
> So... I really don't see any way out of that, which really leaves me with the two
> options that I described in the previous reply.
>
> Summarizing, either:
>  - Adding hwv MMIOs (a bunch of, and each very small) to each clock controller (but
>    still all of them are poking at the same HWV controller, and I foresee that this
>    will backfire in some future iteration of the HWV hardware)
>  - Reverting back to using the "mediatek,hardware-voter" syscon, like done in
>    https://lore.kernel.org/20250624143220.244549-10-laura.nao@collabora.com
>
> I tried really hard and thought about this for weeks (actually, started even before
> your feedback on Laura's series), but now I'm out of practical options that are
> both correctly representing the hardware and not making the implementation fragile
> (or actually more fragile than the actually broken HW implementation's fragility,
> anyway).
>
> And besides - re-reading what I wrote after a bunch of days, the first option of
> adding a bunch of hwv mmios to all of the clock controllers is, in my opinion, a
> (dirty) hack - because those mmios don't belong to the clock controllers, and would
> again misrepresent the hardware in DT - especially keeping in mind the fact that
> the clock controllers can be controlled with *and* (not or) without the HWV (and in
> some instances, even if using HWV, we must still write to the clock controllers'
> mmio for extra programming, as explained before).
>
> Every second I think about this I get more and more convinced that my way of
> passing the SCPSYS-HWV system controller handle as a syscon is right.
>
> Angelo
>

I’ve given this some more thought over the past few days.

I can't see of any other viable alternative either, other than splitting 
up the HWV MMIOs into multiple tiny reg entries across each consumer 
node as mentioned..which feels fragile and wouldn't really be an accurate 
representation of the HW, given they all ultimately target the same HWV MCU.

All considered, modeling the HWV as a shared syscon passed to both clock 
and power controllers still seems to me like the more accurate 
representation of this specific hw layout, given both types of 
controllers interact with the same voter logic and MMIO region.

Krzysztof, does the approach with scattered small reg entries per clock 
controller seem as awkward to you as it does to us? are there any other 
directions you think we should explore here?

Best,

Laura

Re: [RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)

[Chen-Yu Tsai]

On Fri, Jul 11, 2025 at 12:28 AM Laura Nao <laura.nao@collabora.com> wrote:
>
> On 7/7/25 12:40, AngeloGioacchino Del Regno wrote:
> > Il 03/07/25 10:56, AngeloGioacchino Del Regno ha scritto:
> >> Il 02/07/25 08:50, Krzysztof Kozlowski ha scritto:
> >>> On Tue, Jul 01, 2025 at 05:11:48PM +0200, AngeloGioacchino Del Regno wrote:
> >>>> Add documentation for the new MediaTek Hardware Voter, found in
> >>>> MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
> >>>> the MT6991 Dimensity 9400 for Smartphones.
> >>>>
> >>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >>>> ---
> >>>>   .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
> >>>>   1 file changed, 70 insertions(+)
> >>>>   create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991- hardware-voter.yaml
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/firmware/mediatek,mt6991- hardware-voter.yaml b/Documentation/devicetree/bindings/firmware/ mediatek,mt6991-hardware-voter.yaml
> >>>> new file mode 100644
> >>>> index 000000000000..173b74c23a91
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware- voter.yaml
> >>>> @@ -0,0 +1,70 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>>> +# Copyright 2025 Collabora Ltd
> >>>> +%YAML 1.2
> >>>> +---
> >>>> +$id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
> >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>> +
> >>>> +title: MediaTek Hardware Voter (HWV)
> >>>> +
> >>>> +maintainers:
> >>>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >>>> +
> >>>> +description:
> >>>> +  The MediaTek Hardware Voter (HWV) is a SoC-internal fixed-function MCU
> >>>> +  used to collect votes from both the Application Processor and from the
> >>>> +  various other remote processors present in the SoC, and transparently
> >>>> +  turn on or off various hardware resources (for example, power domains
> >>>> +  or system clocks) based on aggregation of votes done in the HWV MCU's
> >>>> +  internal state machine, therefore guaranteeing synchronization of the
> >>>> +  hardware resource requests between all components of the SoC and hence
> >>>> +  avoiding, for example, unclocked or unpowered access to the hardware.
> >>>> +
> >>>> +properties:
> >>>> +  $nodename:
> >>>> +    pattern: "^system-controller@[0-9a-f]+$"
> >>>> +
> >>>> +  compatible:
> >>>> +    const: mediatek,mt6991-hardware-voter
> >>>> +
> >>>> +  reg:
> >>>> +    items:
> >>>> +      - description: Address and size of the Hardware Voter MMIO
> >>>> +
> >>>
> >>> No resources here, so this should go to power controller
> >>>
> >>>> +  power-controller:
> >>>> +    $ref: /schemas/power/mediatek,power-controller.yaml
> >>>> +
> >>>> +required:
> >>>> +  - compatible
> >>>> +  - reg
> >>>> +
> >>>> +additionalProperties: true
> >>>> +
> >>>> +examples:
> >>>> + - |
> >>>> +   scp_hwv: system-controller@14500000 {
> >>>> +     compatible = "mediatek,mt6991-hardware-voter";
> >>>> +     reg = <0 0x14500000 0 0x3000>;
> >>>> +
> >>>> +     power-controller {
> >>>> +       compatible = "mediatek,mt8196-hwv-scp-power-controller";
> >>>
> >>> mt8196 in mt6991 is very confusing.
> >>>
> >>
> >> Yeah that wasn't intentional; fyi, it's almost the same soc, that's why I mixed
> >> them up... :-)
> >>
> >>> Anyway, this does not address my comment at all. You again create some
> >>> sort of syscon for voting, so no. You are supposed to use generic API
> >>> for voting: clocks, power domains, interconnects - whatever is there
> >>> applicable or necessary.
> >>>
> >>
> >> Making that loud and clear: Interconnect is not applicable.
> >>
> >> The only way to do what you're proposing would be to add a bunch of `reg`
> >> to each devicetree node for each clock controller and each power controller;
> >> I can do that, but looks a bit dirty - and still yet another syscon-like
> >> alternative, but without having a real syscon declared in there.
> >>
> >> Mind you - both clock and power controllers are writing both to their own
> >> register space (and enabling external regulators, etc, for power domains)
> >> and to the hardware voter MMIO (which means that the HWV, in hardware, is
> >> fundamentally broken).
> >>
> >> After this reply, the only option that is left to me is the following:
> >>
> >>          topckgen: clock-controller@10000000 {
> >>              compatible = "mediatek,mt8196-topckgen", "syscon";
> >>              reg = <0 0x10000000 0 0x800>, <0 0x14500010 0 0x48>,
> >>                    <0 0x14502c08 0 0x24>;
> >>              reg-names = "base", "hwvoter-base", "hwvoter-status";
> >>              #clock-cells = <1>;
> >>          };
> >>
> >>          imp_iic_wrap_north: clock-controller@13c30000 {
> >>              compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
> >>              reg = <0 0x13c30000 0 0x1000>, <0 0x14500000 0 0xc>,
> >>                    <0 0x14502c00 0 0xc>;
> >>              reg-names = "base", "hwvoter-base", "hwvoter-status";
> >>              #clock-cells = <1>;
> >>          };
> >>
> >>          /* Power Manager with Hardware Voter */
> >>          spm_hwv: power-controller@14500218 {
> >>              compatible = "mediatek,mt8196-hwv-scp-power-controller";
> >>              reg = <0 0x14500218 0 0x20>, <0 0x14501410 0 0x20>,
> >>                    <0 0x14505514 0 0xc>;
> >>              reg-names = "hwvoter-base", "hwvoter-status", "hwvoter-ack";
> >>              #address-cells = <1>;
> >>              #size-cells = <0>;
> >>              #power-domain-cells = <1>;
> >>
> >>              /* SCPSYS hardware voter power domains */
> >>              mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
> >>                  ..... etc, all power domains
> >>
> >> At this point, I'm really not sure that this would be better than just passing
> >> the mediatek,hardware-voter syscon to the clock controllers - as what I've done
> >> previously was effectively representing the hardware in the devicetree as it is,
> >> matching the real HW layout 1:1 (because again, each of the whole HWV MCU(s) are
> >> embedded into each of the two power controllers, one for System power, and one
> >> for Multimedia power).
> >>
> >> (btw, hardware speaking, the power controller is child of a system controller:
> >> there are two system controllers - "scpsystem" is for "compute part", and the
> >> "hfrpsystem" is for the "multimedia part" of the soc).
> >>
> >>   _______________________________________
> >> |                                       |
> >> | SYSTEM CONTROLLER (SCPSYS or HFRPSYS) |
> >> |   _____________________               |
> >> |  |                     |              | <===> Clock Controllers (more than one)
> >> |  | Power Controller    |     SOME     |       (provide subsystem clocks for iso
> >> |  |                     |    OTHER     |        during power domain enablement
> >> |  |     ______________  |   BLOCKS     |        even if a PD is voted)
> >> |  |    |              | |              |       non-subsystem clocks are voted,
> >> |  |    | HW Voter MCU | |              |       but subsystem ones are not voted
> >> |  |    |______________| |              |
> >> |  |_____________________|              | ===> Rest of the SoC
> >> |_______________________________________|
> >>
> >>
> >> Hence I'm asking you - does your idea still stand?
> >>
> >> Because after this, sorry for that - this doesn't want to be an attack - but
> >> I'm starting to have doubts about an approach that doesn't involve syscons.
> >>
> >> Cheers,
> >> Angelo
> >
> > Sorry for the double reply, wanted to add some more words :-)
> >
> > As a note, I also thought about doing the following:
> >
> >     /* Secondary SCPSYS block with HWV capabilities */
> >     scp1_hwv: system-controller@14500000 {
> >         compatible = "mediatek,mt8196-scpsys", "syscon", "simple-mfd";
> >         reg = <0 0x14500000 0 0x3000>;
> >
> >         /* SCP Power Manager with Hardware Voter */
> >         spm_hwv: power-controller {
> >             compatible = "mediatek,mt8196-hwv-scp-power-controller";
> >             #address-cells = <1>;
> >             #size-cells = <0>;
> >             #power-domain-cells = <1>;
> >
> >             /* SCPSYS hardware voter power domains */
> >             mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
> >             ..... etc etc
> >             };
> >         };
> >
> >         imp_iic_wrap_north: clock-controller@13c30000 {
> >             compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
> >             reg = <0 0x13c30000 0 0x1000>;
> >             #clock-cells = <1>;
> >         };
> >     };
> >
> > ...but that's also not applicable, because the clock controllers are physically
> > *not* inside of the scpsys1 block, so that would *also* misrepresent the hardware
> > in the devicetree (besides still using a syscon in a way or another).
> >
> > So... I really don't see any way out of that, which really leaves me with the two
> > options that I described in the previous reply.
> >
> > Summarizing, either:
> >  - Adding hwv MMIOs (a bunch of, and each very small) to each clock controller (but
> >    still all of them are poking at the same HWV controller, and I foresee that this
> >    will backfire in some future iteration of the HWV hardware)
> >  - Reverting back to using the "mediatek,hardware-voter" syscon, like done in
> >    https://lore.kernel.org/20250624143220.244549-10-laura.nao@collabora.com
> >
> > I tried really hard and thought about this for weeks (actually, started even before
> > your feedback on Laura's series), but now I'm out of practical options that are
> > both correctly representing the hardware and not making the implementation fragile
> > (or actually more fragile than the actually broken HW implementation's fragility,
> > anyway).
> >
> > And besides - re-reading what I wrote after a bunch of days, the first option of
> > adding a bunch of hwv mmios to all of the clock controllers is, in my opinion, a
> > (dirty) hack - because those mmios don't belong to the clock controllers, and would
> > again misrepresent the hardware in DT - especially keeping in mind the fact that
> > the clock controllers can be controlled with *and* (not or) without the HWV (and in
> > some instances, even if using HWV, we must still write to the clock controllers'
> > mmio for extra programming, as explained before).
> >
> > Every second I think about this I get more and more convinced that my way of
> > passing the SCPSYS-HWV system controller handle as a syscon is right.
> >
> > Angelo
> >
>
> I’ve given this some more thought over the past few days.
>
> I can't see of any other viable alternative either, other than splitting
> up the HWV MMIOs into multiple tiny reg entries across each consumer
> node as mentioned..which feels fragile and wouldn't really be an accurate
> representation of the HW, given they all ultimately target the same HWV MCU.
>
> All considered, modeling the HWV as a shared syscon passed to both clock
> and power controllers still seems to me like the more accurate
> representation of this specific hw layout, given both types of
> controllers interact with the same voter logic and MMIO region.
>
> Krzysztof, does the approach with scattered small reg entries per clock
> controller seem as awkward to you as it does to us? are there any other
> directions you think we should explore here?

Just to add to the argument, the hardware voter is really just an
alternative interface to the same clock or power controller; it is
not a separate controller. Trying to model it as such means you end
up with two devices fighting over control over the same hardware.

It really should be *one* clock controller with an alternative address
space. How we choose to model the hardware voter address space is up
for debate as Laura mentioned, but modeling it as another clock
controller is misrepresenting the hardware at best, and causing more
confusion for the implementation at worst.

ChenYu

Re: [RFC PATCH 2/3] dt-bindings: firmware: Document the MediaTek Hardware Voter (HWV)

[AngeloGioacchino Del Regno]

Il 15/07/25 06:33, Chen-Yu Tsai ha scritto:
> On Fri, Jul 11, 2025 at 12:28 AM Laura Nao <laura.nao@collabora.com> wrote:
>>
>> On 7/7/25 12:40, AngeloGioacchino Del Regno wrote:
>>> Il 03/07/25 10:56, AngeloGioacchino Del Regno ha scritto:
>>>> Il 02/07/25 08:50, Krzysztof Kozlowski ha scritto:
>>>>> On Tue, Jul 01, 2025 at 05:11:48PM +0200, AngeloGioacchino Del Regno wrote:
>>>>>> Add documentation for the new MediaTek Hardware Voter, found in
>>>>>> MediaTek SoCs like the MT8196 Kompanio Ultra for Chromebooks and
>>>>>> the MT6991 Dimensity 9400 for Smartphones.
>>>>>>
>>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>>>> ---
>>>>>>    .../mediatek,mt6991-hardware-voter.yaml       | 70 +++++++++++++++++++
>>>>>>    1 file changed, 70 insertions(+)
>>>>>>    create mode 100644 Documentation/devicetree/bindings/firmware/mediatek,mt6991- hardware-voter.yaml
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/firmware/mediatek,mt6991- hardware-voter.yaml b/Documentation/devicetree/bindings/firmware/ mediatek,mt6991-hardware-voter.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..173b74c23a91
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/firmware/mediatek,mt6991-hardware- voter.yaml
>>>>>> @@ -0,0 +1,70 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +# Copyright 2025 Collabora Ltd
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/firmware/mediatek,mt6991-hardware-voter.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: MediaTek Hardware Voter (HWV)
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>>>> +
>>>>>> +description:
>>>>>> +  The MediaTek Hardware Voter (HWV) is a SoC-internal fixed-function MCU
>>>>>> +  used to collect votes from both the Application Processor and from the
>>>>>> +  various other remote processors present in the SoC, and transparently
>>>>>> +  turn on or off various hardware resources (for example, power domains
>>>>>> +  or system clocks) based on aggregation of votes done in the HWV MCU's
>>>>>> +  internal state machine, therefore guaranteeing synchronization of the
>>>>>> +  hardware resource requests between all components of the SoC and hence
>>>>>> +  avoiding, for example, unclocked or unpowered access to the hardware.
>>>>>> +
>>>>>> +properties:
>>>>>> +  $nodename:
>>>>>> +    pattern: "^system-controller@[0-9a-f]+$"
>>>>>> +
>>>>>> +  compatible:
>>>>>> +    const: mediatek,mt6991-hardware-voter
>>>>>> +
>>>>>> +  reg:
>>>>>> +    items:
>>>>>> +      - description: Address and size of the Hardware Voter MMIO
>>>>>> +
>>>>>
>>>>> No resources here, so this should go to power controller
>>>>>
>>>>>> +  power-controller:
>>>>>> +    $ref: /schemas/power/mediatek,power-controller.yaml
>>>>>> +
>>>>>> +required:
>>>>>> +  - compatible
>>>>>> +  - reg
>>>>>> +
>>>>>> +additionalProperties: true
>>>>>> +
>>>>>> +examples:
>>>>>> + - |
>>>>>> +   scp_hwv: system-controller@14500000 {
>>>>>> +     compatible = "mediatek,mt6991-hardware-voter";
>>>>>> +     reg = <0 0x14500000 0 0x3000>;
>>>>>> +
>>>>>> +     power-controller {
>>>>>> +       compatible = "mediatek,mt8196-hwv-scp-power-controller";
>>>>>
>>>>> mt8196 in mt6991 is very confusing.
>>>>>
>>>>
>>>> Yeah that wasn't intentional; fyi, it's almost the same soc, that's why I mixed
>>>> them up... :-)
>>>>
>>>>> Anyway, this does not address my comment at all. You again create some
>>>>> sort of syscon for voting, so no. You are supposed to use generic API
>>>>> for voting: clocks, power domains, interconnects - whatever is there
>>>>> applicable or necessary.
>>>>>
>>>>
>>>> Making that loud and clear: Interconnect is not applicable.
>>>>
>>>> The only way to do what you're proposing would be to add a bunch of `reg`
>>>> to each devicetree node for each clock controller and each power controller;
>>>> I can do that, but looks a bit dirty - and still yet another syscon-like
>>>> alternative, but without having a real syscon declared in there.
>>>>
>>>> Mind you - both clock and power controllers are writing both to their own
>>>> register space (and enabling external regulators, etc, for power domains)
>>>> and to the hardware voter MMIO (which means that the HWV, in hardware, is
>>>> fundamentally broken).
>>>>
>>>> After this reply, the only option that is left to me is the following:
>>>>
>>>>           topckgen: clock-controller@10000000 {
>>>>               compatible = "mediatek,mt8196-topckgen", "syscon";
>>>>               reg = <0 0x10000000 0 0x800>, <0 0x14500010 0 0x48>,
>>>>                     <0 0x14502c08 0 0x24>;
>>>>               reg-names = "base", "hwvoter-base", "hwvoter-status";
>>>>               #clock-cells = <1>;
>>>>           };
>>>>
>>>>           imp_iic_wrap_north: clock-controller@13c30000 {
>>>>               compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
>>>>               reg = <0 0x13c30000 0 0x1000>, <0 0x14500000 0 0xc>,
>>>>                     <0 0x14502c00 0 0xc>;
>>>>               reg-names = "base", "hwvoter-base", "hwvoter-status";
>>>>               #clock-cells = <1>;
>>>>           };
>>>>
>>>>           /* Power Manager with Hardware Voter */
>>>>           spm_hwv: power-controller@14500218 {
>>>>               compatible = "mediatek,mt8196-hwv-scp-power-controller";
>>>>               reg = <0 0x14500218 0 0x20>, <0 0x14501410 0 0x20>,
>>>>                     <0 0x14505514 0 0xc>;
>>>>               reg-names = "hwvoter-base", "hwvoter-status", "hwvoter-ack";
>>>>               #address-cells = <1>;
>>>>               #size-cells = <0>;
>>>>               #power-domain-cells = <1>;
>>>>
>>>>               /* SCPSYS hardware voter power domains */
>>>>               mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
>>>>                   ..... etc, all power domains
>>>>
>>>> At this point, I'm really not sure that this would be better than just passing
>>>> the mediatek,hardware-voter syscon to the clock controllers - as what I've done
>>>> previously was effectively representing the hardware in the devicetree as it is,
>>>> matching the real HW layout 1:1 (because again, each of the whole HWV MCU(s) are
>>>> embedded into each of the two power controllers, one for System power, and one
>>>> for Multimedia power).
>>>>
>>>> (btw, hardware speaking, the power controller is child of a system controller:
>>>> there are two system controllers - "scpsystem" is for "compute part", and the
>>>> "hfrpsystem" is for the "multimedia part" of the soc).
>>>>
>>>>    _______________________________________
>>>> |                                       |
>>>> | SYSTEM CONTROLLER (SCPSYS or HFRPSYS) |
>>>> |   _____________________               |
>>>> |  |                     |              | <===> Clock Controllers (more than one)
>>>> |  | Power Controller    |     SOME     |       (provide subsystem clocks for iso
>>>> |  |                     |    OTHER     |        during power domain enablement
>>>> |  |     ______________  |   BLOCKS     |        even if a PD is voted)
>>>> |  |    |              | |              |       non-subsystem clocks are voted,
>>>> |  |    | HW Voter MCU | |              |       but subsystem ones are not voted
>>>> |  |    |______________| |              |
>>>> |  |_____________________|              | ===> Rest of the SoC
>>>> |_______________________________________|
>>>>
>>>>
>>>> Hence I'm asking you - does your idea still stand?
>>>>
>>>> Because after this, sorry for that - this doesn't want to be an attack - but
>>>> I'm starting to have doubts about an approach that doesn't involve syscons.
>>>>
>>>> Cheers,
>>>> Angelo
>>>
>>> Sorry for the double reply, wanted to add some more words :-)
>>>
>>> As a note, I also thought about doing the following:
>>>
>>>      /* Secondary SCPSYS block with HWV capabilities */
>>>      scp1_hwv: system-controller@14500000 {
>>>          compatible = "mediatek,mt8196-scpsys", "syscon", "simple-mfd";
>>>          reg = <0 0x14500000 0 0x3000>;
>>>
>>>          /* SCP Power Manager with Hardware Voter */
>>>          spm_hwv: power-controller {
>>>              compatible = "mediatek,mt8196-hwv-scp-power-controller";
>>>              #address-cells = <1>;
>>>              #size-cells = <0>;
>>>              #power-domain-cells = <1>;
>>>
>>>              /* SCPSYS hardware voter power domains */
>>>              mm_proc_dormant: power-domain@MT8196_POWER_DOMAIN_MM_PROC_DORMANT {
>>>              ..... etc etc
>>>              };
>>>          };
>>>
>>>          imp_iic_wrap_north: clock-controller@13c30000 {
>>>              compatible = "mediatek,mt8196-imp-iic-wrap-n", "syscon";
>>>              reg = <0 0x13c30000 0 0x1000>;
>>>              #clock-cells = <1>;
>>>          };
>>>      };
>>>
>>> ...but that's also not applicable, because the clock controllers are physically
>>> *not* inside of the scpsys1 block, so that would *also* misrepresent the hardware
>>> in the devicetree (besides still using a syscon in a way or another).
>>>
>>> So... I really don't see any way out of that, which really leaves me with the two
>>> options that I described in the previous reply.
>>>
>>> Summarizing, either:
>>>   - Adding hwv MMIOs (a bunch of, and each very small) to each clock controller (but
>>>     still all of them are poking at the same HWV controller, and I foresee that this
>>>     will backfire in some future iteration of the HWV hardware)
>>>   - Reverting back to using the "mediatek,hardware-voter" syscon, like done in
>>>     https://lore.kernel.org/20250624143220.244549-10-laura.nao@collabora.com
>>>
>>> I tried really hard and thought about this for weeks (actually, started even before
>>> your feedback on Laura's series), but now I'm out of practical options that are
>>> both correctly representing the hardware and not making the implementation fragile
>>> (or actually more fragile than the actually broken HW implementation's fragility,
>>> anyway).
>>>
>>> And besides - re-reading what I wrote after a bunch of days, the first option of
>>> adding a bunch of hwv mmios to all of the clock controllers is, in my opinion, a
>>> (dirty) hack - because those mmios don't belong to the clock controllers, and would
>>> again misrepresent the hardware in DT - especially keeping in mind the fact that
>>> the clock controllers can be controlled with *and* (not or) without the HWV (and in
>>> some instances, even if using HWV, we must still write to the clock controllers'
>>> mmio for extra programming, as explained before).
>>>
>>> Every second I think about this I get more and more convinced that my way of
>>> passing the SCPSYS-HWV system controller handle as a syscon is right.
>>>
>>> Angelo
>>>
>>
>> I’ve given this some more thought over the past few days.
>>
>> I can't see of any other viable alternative either, other than splitting
>> up the HWV MMIOs into multiple tiny reg entries across each consumer
>> node as mentioned..which feels fragile and wouldn't really be an accurate
>> representation of the HW, given they all ultimately target the same HWV MCU.
>>
>> All considered, modeling the HWV as a shared syscon passed to both clock
>> and power controllers still seems to me like the more accurate
>> representation of this specific hw layout, given both types of
>> controllers interact with the same voter logic and MMIO region.
>>
>> Krzysztof, does the approach with scattered small reg entries per clock
>> controller seem as awkward to you as it does to us? are there any other
>> directions you think we should explore here?
> 
> Just to add to the argument, the hardware voter is really just an
> alternative interface to the same clock or power controller; it is
> not a separate controller. Trying to model it as such means you end
> up with two devices fighting over control over the same hardware.
> 
> It really should be *one* clock controller with an alternative address
> space. How we choose to model the hardware voter address space is up
> for debate as Laura mentioned, but modeling it as another clock
> controller is misrepresenting the hardware at best, and causing more
> confusion for the implementation at worst.
> 
> ChenYu

Alright.
Let's go back to the previously proposed "mediatek,hardware-voter" way then.

Regarding that to be a single clock controller, I'm afraid that won't really be
possible because not all clocks in a single domain are voted - especially the
mux-gate clocks can't really be put in one single clock controller because to
manage those we have to write to the HWV *and* to the clock controller MMIO in the
same operation in order to actually manage the clock.

If that wasn't the case, I would've been happier to have all of the HWV clocks in
one clock controller (even though those belong to different SoC subsystems)...
...but let's see if we can (and if it makes sense to) aggregate at least some!

Thanks everyone!

Cheers,
Angelo