[PATCH v2 0/4] allwinner: a523: Add power controllers

[PATCH v2 0/4] allwinner: a523: Add power controllers

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

Hi folks,

This is v2 of my A523 power controllers series.

Changes since v1:
- Re-order compatible string entries
- Fix name of header file to match compatible string
- Link to v1:
  https://lore.kernel.org/all/20250627152918.2606728-1-wens@kernel.org/

This series adds the power controllers found in the Allwinner A523
family of SoCs. There are two power controllers. One is the same type
as those found in the D1 SoC, just with a different number of valid
power domains. The second is (I assume) a unit based on ARM's PCK-600
power controller. Some of the registers and values match up, but there
are extra registers for delay controls in the PCK-600's reserved
register range.

Patch 1 adds new compatible string entries for both of these
controllers.

Patch 2 adds support for the A523 PPU to the existing D1 PPU driver.

Patch 3 adds a new driver of the PCK-600 unit in the A523 SoC.

Patch 4 adds device nodes for both of these controllers.


Please have a look. The power controllers are critical for enabling more
peripherals, such as display output, camera input, video codecs, the NPU,
and a second DWMAC-compatible ethernet interface.


Thanks
ChenYu


Chen-Yu Tsai (4):
  dt-bindings: power: Add A523 PPU and PCK600 power controllers
  pmdomain: sunxi: sun20i-ppu: add A523 support
  pmdomain: sunxi: add driver for Allwinner A523's PCK-600 power
    controller
  arm64: dts: allwinner: a523: Add power controller device nodes

 .../power/allwinner,sun20i-d1-ppu.yaml        |   4 +-
 .../arm64/boot/dts/allwinner/sun55i-a523.dtsi |  18 ++
 drivers/pmdomain/sunxi/Kconfig                |   8 +
 drivers/pmdomain/sunxi/Makefile               |   1 +
 drivers/pmdomain/sunxi/sun20i-ppu.c           |  17 ++
 drivers/pmdomain/sunxi/sun55i-pck600.c        | 225 ++++++++++++++++++
 .../power/allwinner,sun55i-a523-pck-600.h     |  15 ++
 .../power/allwinner,sun55i-a523-ppu.h         |  12 +
 8 files changed, 299 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pmdomain/sunxi/sun55i-pck600.c
 create mode 100644 include/dt-bindings/power/allwinner,sun55i-a523-pck-600.h
 create mode 100644 include/dt-bindings/power/allwinner,sun55i-a523-ppu.h

-- 
2.39.5

[PATCH v2 1/4] dt-bindings: power: Add A523 PPU and PCK600 power controllers

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

The A523 PPU is likely the same kind of hardware seen on previous SoCs.

The A523 PCK600, as the name suggests, is likely a customized version
of ARM's PCK-600 power controller. Comparing the BSP driver against
ARM's PPU datasheet shows that the basic registers line up, but
Allwinner's hardware has some additional delay controls in the reserved
register range. As such it is likely not fully compatible with the
standard ARM version.

Document A523 PPU and PCK600 compatibles.

Also reorder the compatible string entries so they are grouped and
ordered by family first, then by SoC model.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Chen-Yu Tsai <wens@csie.org>

---
Changes since v1:
- Re-order compatible string entries
- Fix name of header file to match compatible string
---
 .../bindings/power/allwinner,sun20i-d1-ppu.yaml   |  4 +++-
 .../power/allwinner,sun55i-a523-pck-600.h         | 15 +++++++++++++++
 .../dt-bindings/power/allwinner,sun55i-a523-ppu.h | 12 ++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/power/allwinner,sun55i-a523-pck-600.h
 create mode 100644 include/dt-bindings/power/allwinner,sun55i-a523-ppu.h

diff --git a/Documentation/devicetree/bindings/power/allwinner,sun20i-d1-ppu.yaml b/Documentation/devicetree/bindings/power/allwinner,sun20i-d1-ppu.yaml
index f578be6a3bc8..a28e75a9cb6a 100644
--- a/Documentation/devicetree/bindings/power/allwinner,sun20i-d1-ppu.yaml
+++ b/Documentation/devicetree/bindings/power/allwinner,sun20i-d1-ppu.yaml
@@ -16,8 +16,10 @@ description:
 properties:
   compatible:
     enum:
-      - allwinner,sun20i-d1-ppu
       - allwinner,sun8i-v853-ppu
+      - allwinner,sun20i-d1-ppu
+      - allwinner,sun55i-a523-pck-600
+      - allwinner,sun55i-a523-ppu
 
   reg:
     maxItems: 1
diff --git a/include/dt-bindings/power/allwinner,sun55i-a523-pck-600.h b/include/dt-bindings/power/allwinner,sun55i-a523-pck-600.h
new file mode 100644
index 000000000000..6b3d8ea7bb69
--- /dev/null
+++ b/include/dt-bindings/power/allwinner,sun55i-a523-pck-600.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_POWER_SUN55I_A523_PCK600_H_
+#define _DT_BINDINGS_POWER_SUN55I_A523_PCK600_H_
+
+#define PD_VE			0
+#define PD_GPU			1
+#define PD_VI			2
+#define PD_VO0			3
+#define PD_VO1			4
+#define PD_DE			5
+#define PD_NAND			6
+#define PD_PCIE			7
+
+#endif /* _DT_BINDINGS_POWER_SUN55I_A523_PCK600_H_ */
diff --git a/include/dt-bindings/power/allwinner,sun55i-a523-ppu.h b/include/dt-bindings/power/allwinner,sun55i-a523-ppu.h
new file mode 100644
index 000000000000..bc9aba73c19a
--- /dev/null
+++ b/include/dt-bindings/power/allwinner,sun55i-a523-ppu.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_POWER_SUN55I_A523_PPU_H_
+#define _DT_BINDINGS_POWER_SUN55I_A523_PPU_H_
+
+#define PD_DSP			0
+#define PD_NPU			1
+#define PD_AUDIO		2
+#define PD_SRAM			3
+#define PD_RISCV		4
+
+#endif /* _DT_BINDINGS_POWER_SUN55I_A523_PPU_H_ */
-- 
2.39.5

[PATCH v2 2/4] pmdomain: sunxi: sun20i-ppu: add A523 support

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

A523 has a PPU like the one in the Allwinner D1 SoC.

Add a compatible entry and a list of power domain names for it.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/pmdomain/sunxi/sun20i-ppu.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pmdomain/sunxi/sun20i-ppu.c b/drivers/pmdomain/sunxi/sun20i-ppu.c
index 9f002748d224..b65876a68cc1 100644
--- a/drivers/pmdomain/sunxi/sun20i-ppu.c
+++ b/drivers/pmdomain/sunxi/sun20i-ppu.c
@@ -193,6 +193,19 @@ static const struct sun20i_ppu_desc sun8i_v853_ppu_desc = {
 	.num_domains	= ARRAY_SIZE(sun8i_v853_ppu_pd_names),
 };
 
+static const char *const sun55i_a523_ppu_pd_names[] = {
+	"DSP",
+	"NPU",
+	"AUDIO",
+	"SRAM",
+	"RISCV",
+};
+
+static const struct sun20i_ppu_desc sun55i_a523_ppu_desc = {
+	.names		= sun55i_a523_ppu_pd_names,
+	.num_domains	= ARRAY_SIZE(sun55i_a523_ppu_pd_names),
+};
+
 static const struct of_device_id sun20i_ppu_of_match[] = {
 	{
 		.compatible	= "allwinner,sun20i-d1-ppu",
@@ -202,6 +215,10 @@ static const struct of_device_id sun20i_ppu_of_match[] = {
 		.compatible	= "allwinner,sun8i-v853-ppu",
 		.data		= &sun8i_v853_ppu_desc,
 	},
+	{
+		.compatible	= "allwinner,sun55i-a523-ppu",
+		.data		= &sun55i_a523_ppu_desc,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun20i_ppu_of_match);
-- 
2.39.5

[PATCH v2 3/4] pmdomain: sunxi: add driver for Allwinner A523's PCK-600 power controller

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

Allwinner A523 family has a second power controller, named PCK-600 in
the datasheets and BSP. It is likely based on ARM's PCK-600 hardware
block, with some additional delay controls. The only documentation for
this hardware is the BSP driver. The standard registers defined in ARM's
Power Policy Unit Architecture Specification line up. Some extra delay
controls are found in the reserved range of registers.

Add a driver for this power controller. Delay control register values
and power domain names are from the BSP driver.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/pmdomain/sunxi/Kconfig         |   8 +
 drivers/pmdomain/sunxi/Makefile        |   1 +
 drivers/pmdomain/sunxi/sun55i-pck600.c | 225 +++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/pmdomain/sunxi/sun55i-pck600.c

diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
index 43eecb3ea981..3e2b77cd9a2b 100644
--- a/drivers/pmdomain/sunxi/Kconfig
+++ b/drivers/pmdomain/sunxi/Kconfig
@@ -18,3 +18,11 @@ config SUN50I_H6_PRCM_PPU
 	  Say y to enable the Allwinner H6/H616 PRCM power domain driver.
 	  This is required to enable the Mali GPU in the H616 SoC, it is
 	  optional for the H6.
+
+config SUN55I_PCK600
+	bool "Allwinner A523 PCK-600 power domain driver"
+	depends on PM
+	select PM_GENERIC_DOMAINS
+	help
+	  Say y to enable the PCK-600 power domain driver. This saves power
+	  when certain peripherals, such as the video engine, are idle.
diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
index c1343e123759..e344b232fc9f 100644
--- a/drivers/pmdomain/sunxi/Makefile
+++ b/drivers/pmdomain/sunxi/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_SUN20I_PPU)		+= sun20i-ppu.o
 obj-$(CONFIG_SUN50I_H6_PRCM_PPU)	+= sun50i-h6-prcm-ppu.o
+obj-$(CONFIG_SUN55I_PCK600)		+= sun55i-pck600.o
diff --git a/drivers/pmdomain/sunxi/sun55i-pck600.c b/drivers/pmdomain/sunxi/sun55i-pck600.c
new file mode 100644
index 000000000000..7248f6113665
--- /dev/null
+++ b/drivers/pmdomain/sunxi/sun55i-pck600.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Allwinner PCK-600 power domain support
+ *
+ * Copyright (c) 2025 Chen-Yu Tsai <wens@csie.org>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/string_choices.h>
+
+#define PPU_PWPR    0x0
+#define PPU_PWSR    0x8
+#define	PPU_DCDR0   0x170
+#define PPU_DCDR1   0x174
+
+#define PPU_PWSR_PWR_STATUS	GENMASK(3, 0)
+#define PPU_POWER_MODE_ON	0x8
+#define PPU_POWER_MODE_OFF	0x0
+
+#define PPU_REG_SIZE	0x1000
+
+struct sunxi_pck600_desc {
+	const char * const *pd_names;
+	unsigned int num_domains;
+	u32 logic_power_switch0_delay_offset;
+	u32 logic_power_switch1_delay_offset;
+	u32 off2on_delay_offset;
+	u32 device_ctrl0_delay;
+	u32 device_ctrl1_delay;
+	u32 logic_power_switch0_delay;
+	u32 logic_power_switch1_delay;
+	u32 off2on_delay;
+};
+
+struct sunxi_pck600_pd {
+	struct generic_pm_domain genpd;
+	struct sunxi_pck600 *pck;
+	void __iomem *base;
+};
+
+struct sunxi_pck600 {
+	struct device *dev;
+	struct genpd_onecell_data genpd_data;
+	struct sunxi_pck600_pd pds[];
+};
+
+#define to_sunxi_pd(gpd) container_of(gpd, struct sunxi_pck600_pd, genpd)
+
+static int sunxi_pck600_pd_set_power(struct sunxi_pck600_pd *pd, bool on)
+{
+	struct sunxi_pck600 *pck = pd->pck;
+	struct generic_pm_domain *genpd = &pd->genpd;
+	int ret;
+	u32 val, reg;
+
+	val = on ? PPU_POWER_MODE_ON : PPU_POWER_MODE_OFF;
+
+	reg = readl(pd->base + PPU_PWPR);
+	FIELD_MODIFY(PPU_PWSR_PWR_STATUS, &reg, val);
+	writel(reg, pd->base + PPU_PWPR);
+
+	/* push write out to hardware */
+	reg = readl(pd->base + PPU_PWPR);
+
+	ret = readl_poll_timeout_atomic(pd->base + PPU_PWSR, reg,
+					FIELD_GET(PPU_PWSR_PWR_STATUS, reg) == val,
+					0, 10000);
+	if (ret)
+		dev_err(pck->dev, "failed to turn domain \"%s\" %s: %d\n",
+			genpd->name, str_on_off(on), ret);
+
+	return ret;
+}
+
+static int sunxi_pck600_power_on(struct generic_pm_domain *domain)
+{
+	struct sunxi_pck600_pd *pd = to_sunxi_pd(domain);
+
+	return sunxi_pck600_pd_set_power(pd, true);
+}
+
+static int sunxi_pck600_power_off(struct generic_pm_domain *domain)
+{
+	struct sunxi_pck600_pd *pd = to_sunxi_pd(domain);
+
+	return sunxi_pck600_pd_set_power(pd, false);
+}
+
+static void sunxi_pck600_pd_setup(struct sunxi_pck600_pd *pd,
+				  const struct sunxi_pck600_desc *desc)
+{
+	writel(desc->device_ctrl0_delay, pd->base + PPU_DCDR0);
+	writel(desc->device_ctrl1_delay, pd->base + PPU_DCDR1);
+	writel(desc->logic_power_switch0_delay,
+	       pd->base + desc->logic_power_switch0_delay_offset);
+	writel(desc->logic_power_switch1_delay,
+	       pd->base + desc->logic_power_switch1_delay_offset);
+	writel(desc->off2on_delay, pd->base + desc->off2on_delay_offset);
+}
+
+static int sunxi_pck600_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct sunxi_pck600_desc *desc;
+	struct genpd_onecell_data *genpds;
+	struct sunxi_pck600 *pck;
+	struct reset_control *rst;
+	struct clk *clk;
+	void __iomem *base;
+	int i, ret;
+
+	desc = of_device_get_match_data(dev);
+
+	pck = devm_kzalloc(dev, struct_size(pck, pds, desc->num_domains), GFP_KERNEL);
+	if (!pck)
+		return -ENOMEM;
+
+	pck->dev = &pdev->dev;
+	platform_set_drvdata(pdev, pck);
+
+	genpds = &pck->genpd_data;
+	genpds->num_domains = desc->num_domains;
+	genpds->domains = devm_kcalloc(dev, desc->num_domains,
+				       sizeof(*genpds->domains), GFP_KERNEL);
+	if (!genpds->domains)
+		return -ENOMEM;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	rst = devm_reset_control_get_exclusive_released(dev, NULL);
+	if (IS_ERR(rst))
+		return dev_err_probe(dev, PTR_ERR(rst), "failed to get reset control\n");
+
+	clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n");
+
+	for (i = 0; i < desc->num_domains; i++) {
+		struct sunxi_pck600_pd *pd = &pck->pds[i];
+
+		pd->genpd.name = desc->pd_names[i];
+		pd->genpd.power_off = sunxi_pck600_power_off;
+		pd->genpd.power_on = sunxi_pck600_power_on;
+		pd->base = base + PPU_REG_SIZE * i;
+
+		sunxi_pck600_pd_setup(pd, desc);
+		ret = pm_genpd_init(&pd->genpd, NULL, false);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to initialize power domain\n");
+			goto err_remove_pds;
+		}
+
+		genpds->domains[i] = &pd->genpd;
+	}
+
+	ret = of_genpd_add_provider_onecell(dev_of_node(dev), genpds);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to add PD provider\n");
+		goto err_remove_pds;
+	}
+
+	return 0;
+
+err_remove_pds:
+	for (i--; i >= 0; i--)
+		pm_genpd_remove(genpds->domains[i]);
+
+	return ret;
+}
+
+static const char * const sun55i_a523_pck600_pd_names[] = {
+	"VE", "GPU", "VI", "VO0", "VO1", "DE", "NAND", "PCIE"
+};
+
+static const struct sunxi_pck600_desc sun55i_a523_pck600_desc = {
+	.pd_names = sun55i_a523_pck600_pd_names,
+	.num_domains = ARRAY_SIZE(sun55i_a523_pck600_pd_names),
+	.logic_power_switch0_delay_offset = 0xc00,
+	.logic_power_switch1_delay_offset = 0xc04,
+	.off2on_delay_offset = 0xc10,
+	.device_ctrl0_delay = 0xffffff,
+	.device_ctrl1_delay = 0xffff,
+	.logic_power_switch0_delay = 0x8080808,
+	.logic_power_switch1_delay = 0x808,
+	.off2on_delay = 0x8
+};
+
+static const struct of_device_id sunxi_pck600_of_match[] = {
+	{
+		.compatible	= "allwinner,sun55i-a523-pck-600",
+		.data		= &sun55i_a523_pck600_desc,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, sunxi_pck600_of_match);
+
+static struct platform_driver sunxi_pck600_driver = {
+	.probe = sunxi_pck600_probe,
+	.driver = {
+		.name   = "sunxi-pck-600",
+		.of_match_table = sunxi_pck600_of_match,
+		/* Power domains cannot be removed if in use. */
+		.suppress_bind_attrs = true,
+	},
+};
+module_platform_driver(sunxi_pck600_driver);
+
+MODULE_DESCRIPTION("Allwinner PCK-600 power domain driver");
+MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
+MODULE_LICENSE("GPL");
-- 
2.39.5

[PATCH v2 4/4] arm64: dts: allwinner: a523: Add power controller device nodes

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

The A523 SoC family has two power controllers, one based on the existing
PPU, and one newer one based on ARM's PCK-600.

Add device nodes for both of them.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
index cf0bc39aab04..dd6fa22f960f 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
@@ -7,6 +7,8 @@
 #include <dt-bindings/clock/sun55i-a523-r-ccu.h>
 #include <dt-bindings/reset/sun55i-a523-ccu.h>
 #include <dt-bindings/reset/sun55i-a523-r-ccu.h>
+#include <dt-bindings/power/allwinner,sun55i-a523-ppu.h>
+#include <dt-bindings/power/allwinner,sun55i-a523-pck600.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -576,6 +578,14 @@ mdio0: mdio {
 			};
 		};
 
+		ppu: power-controller@7001400 {
+			compatible = "allwinner,sun55i-a523-ppu";
+			reg = <0x07001400 0x400>;
+			clocks = <&r_ccu CLK_BUS_R_PPU1>;
+			resets = <&r_ccu RST_BUS_R_PPU1>;
+			#power-domain-cells = <1>;
+		};
+
 		r_ccu: clock-controller@7010000 {
 			compatible = "allwinner,sun55i-a523-r-ccu";
 			reg = <0x7010000 0x250>;
@@ -622,6 +632,14 @@ r_i2c_pins: r-i2c-pins {
 			};
 		};
 
+		pck600: power-controller@7060000 {
+			compatible = "allwinner,sun55i-a523-pck-600";
+			reg = <0x07060000 0x8000>;
+			clocks = <&r_ccu CLK_BUS_R_PPU0>;
+			resets = <&r_ccu RST_BUS_R_PPU0>;
+			#power-domain-cells = <1>;
+		};
+
 		r_i2c0: i2c@7081400 {
 			compatible = "allwinner,sun55i-a523-i2c",
 				     "allwinner,sun8i-v536-i2c",
-- 
2.39.5

Re: [PATCH v2 0/4] allwinner: a523: Add power controllers

[Rob Herring (Arm)]

On Wed, 09 Jul 2025 23:53:39 +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> Hi folks,
> 
> This is v2 of my A523 power controllers series.
> 
> Changes since v1:
> - Re-order compatible string entries
> - Fix name of header file to match compatible string
> - Link to v1:
>   https://lore.kernel.org/all/20250627152918.2606728-1-wens@kernel.org/
> 
> This series adds the power controllers found in the Allwinner A523
> family of SoCs. There are two power controllers. One is the same type
> as those found in the D1 SoC, just with a different number of valid
> power domains. The second is (I assume) a unit based on ARM's PCK-600
> power controller. Some of the registers and values match up, but there
> are extra registers for delay controls in the PCK-600's reserved
> register range.
> 
> Patch 1 adds new compatible string entries for both of these
> controllers.
> 
> Patch 2 adds support for the A523 PPU to the existing D1 PPU driver.
> 
> Patch 3 adds a new driver of the PCK-600 unit in the A523 SoC.
> 
> Patch 4 adds device nodes for both of these controllers.
> 
> 
> Please have a look. The power controllers are critical for enabling more
> peripherals, such as display output, camera input, video codecs, the NPU,
> and a second DWMAC-compatible ethernet interface.
> 
> 
> Thanks
> ChenYu
> 
> 
> Chen-Yu Tsai (4):
>   dt-bindings: power: Add A523 PPU and PCK600 power controllers
>   pmdomain: sunxi: sun20i-ppu: add A523 support
>   pmdomain: sunxi: add driver for Allwinner A523's PCK-600 power
>     controller
>   arm64: dts: allwinner: a523: Add power controller device nodes
> 
>  .../power/allwinner,sun20i-d1-ppu.yaml        |   4 +-
>  .../arm64/boot/dts/allwinner/sun55i-a523.dtsi |  18 ++
>  drivers/pmdomain/sunxi/Kconfig                |   8 +
>  drivers/pmdomain/sunxi/Makefile               |   1 +
>  drivers/pmdomain/sunxi/sun20i-ppu.c           |  17 ++
>  drivers/pmdomain/sunxi/sun55i-pck600.c        | 225 ++++++++++++++++++
>  .../power/allwinner,sun55i-a523-pck-600.h     |  15 ++
>  .../power/allwinner,sun55i-a523-ppu.h         |  12 +
>  8 files changed, 299 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/pmdomain/sunxi/sun55i-pck600.c
>  create mode 100644 include/dt-bindings/power/allwinner,sun55i-a523-pck-600.h
>  create mode 100644 include/dt-bindings/power/allwinner,sun55i-a523-ppu.h
> 
> --
> 2.39.5
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: attempting to guess base-commit...
 Base: tags/next-20250709 (exact match)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)

New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/allwinner/' for 20250709155343.3765227-1-wens@kernel.org:

In file included from arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dts:8:
arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi:11:10: fatal error: dt-bindings/power/allwinner,sun55i-a523-pck600.h: No such file or directory
   11 | #include <dt-bindings/power/allwinner,sun55i-a523-pck600.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [scripts/Makefile.dtbs:131: arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dtb] Error 1
make[2]: *** [scripts/Makefile.build:554: arch/arm64/boot/dts/allwinner] Error 2
make[2]: Target 'arch/arm64/boot/dts/allwinner/sun55i-t527-orangepi-4a.dtb' not remade because of errors.
make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1478: allwinner/sun55i-t527-orangepi-4a.dtb] Error 2
In file included from arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dts:6:
arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi:11:10: fatal error: dt-bindings/power/allwinner,sun55i-a523-pck600.h: No such file or directory
   11 | #include <dt-bindings/power/allwinner,sun55i-a523-pck600.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [scripts/Makefile.dtbs:131: arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dtb] Error 1
make[2]: *** [scripts/Makefile.build:554: arch/arm64/boot/dts/allwinner] Error 2
make[2]: Target 'arch/arm64/boot/dts/allwinner/sun55i-h728-x96qpro+.dtb' not remade because of errors.
make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1478: allwinner/sun55i-h728-x96qpro+.dtb] Error 2
In file included from arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts:6:
arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi:11:10: fatal error: dt-bindings/power/allwinner,sun55i-a523-pck600.h: No such file or directory
   11 | #include <dt-bindings/power/allwinner,sun55i-a523-pck600.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [scripts/Makefile.dtbs:131: arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dtb] Error 1
make[2]: *** [scripts/Makefile.build:554: arch/arm64/boot/dts/allwinner] Error 2
make[2]: Target 'arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dtb' not remade because of errors.
make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1478: allwinner/sun55i-a527-cubie-a5e.dtb] Error 2
In file included from arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dts:6:
arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi:11:10: fatal error: dt-bindings/power/allwinner,sun55i-a523-pck600.h: No such file or directory
   11 | #include <dt-bindings/power/allwinner,sun55i-a523-pck600.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [scripts/Makefile.dtbs:131: arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dtb] Error 1
make[2]: *** [scripts/Makefile.build:554: arch/arm64/boot/dts/allwinner] Error 2
make[2]: Target 'arch/arm64/boot/dts/allwinner/sun55i-t527-avaota-a1.dtb' not remade because of errors.
make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1478: allwinner/sun55i-t527-avaota-a1.dtb] Error 2
make: *** [Makefile:248: __sub-make] Error 2
make: Target 'allwinner/sun50i-h616-bigtreetech-cb1-manta.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-libretech-all-h3-it.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h6-tanix-tx6.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h616-x96-mate.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-orangepi-pc2.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-libretech-all-h3-cc.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h700-anbernic-rg35xx-plus.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-orangepi-zero-plus2.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pinephone-1.1.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h618-orangepi-zero2w.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h618-longanpi-3h.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h6-pine-h64.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h616-orangepi-zero2.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-olinuxino-emmc.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-sopine-baseboard.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-emlid-neutis-n5-devboard.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h6-beelink-gs1.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h616-bigtreetech-pi.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h700-anbernic-rg35xx-2024.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a100-allwinner-perf1.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-orangepi-win.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pinephone-1.2.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h700-anbernic-rg35xx-h.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pine64.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pinetab-early-adopter.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-nanopi-neo-plus2.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-bananapi-m2-plus-v1.2.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h618-yuzukihd-chameleon.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h6-orangepi-3.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-amarula-relic.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pine64-lts.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-bananapi-m2-plus.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-bananapi-m64.dtb' not remade because of errors.
make: Target 'allwinner/sun55i-t527-orangepi-4a.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-orangepi-zero-plus.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-olinuxino.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pinebook.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-oceanic-5205-5inmfd.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pine64-plus.dtb' not remade because of errors.
make: Target 'allwinner/sun55i-h728-x96qpro+.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-teres-i.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pinetab.dtb' not remade because of errors.
make: Target 'allwinner/sun55i-a527-cubie-a5e.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h700-anbernic-rg35xx-sp.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h6-pine-h64-model-b.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-orangepi-prime.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-nanopi-a64.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h618-transpeed-8k618-t.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-libretech-all-h5-cc.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a64-pinephone-1.0.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h6-orangepi-lite2.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h6-tanix-tx6-mini.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h618-orangepi-zero3.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-nanopi-r1s-h5.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h64-remix-mini-pc.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-a133-liontron-h-a133l.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h5-nanopi-neo2.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h313-tanix-tx1.dtb' not remade because of errors.
make: Target 'allwinner/sun55i-t527-avaota-a1.dtb' not remade because of errors.
make: Target 'allwinner/sun50i-h6-orangepi-one-plus.dtb' not remade because of errors.

Re: [PATCH v2 3/4] pmdomain: sunxi: add driver for Allwinner A523's PCK-600 power controller

[Andre Przywara]

Hi Chen-Yu,

thanks for posting this! This is a quick first view, haven't compared 
against the BSP bits yet....

On 09/07/2025 16:53, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> Allwinner A523 family has a second power controller, named PCK-600 in
> the datasheets and BSP. It is likely based on ARM's PCK-600 hardware
> block, with some additional delay controls. The only documentation for
> this hardware is the BSP driver. The standard registers defined in ARM's
> Power Policy Unit Architecture Specification line up. Some extra delay
> controls are found in the reserved range of registers.
> 
> Add a driver for this power controller. Delay control register values
> and power domain names are from the BSP driver.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>   drivers/pmdomain/sunxi/Kconfig         |   8 +
>   drivers/pmdomain/sunxi/Makefile        |   1 +
>   drivers/pmdomain/sunxi/sun55i-pck600.c | 225 +++++++++++++++++++++++++
>   3 files changed, 234 insertions(+)
>   create mode 100644 drivers/pmdomain/sunxi/sun55i-pck600.c
> 
> diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
> index 43eecb3ea981..3e2b77cd9a2b 100644
> --- a/drivers/pmdomain/sunxi/Kconfig
> +++ b/drivers/pmdomain/sunxi/Kconfig
> @@ -18,3 +18,11 @@ config SUN50I_H6_PRCM_PPU
>   	  Say y to enable the Allwinner H6/H616 PRCM power domain driver.
>   	  This is required to enable the Mali GPU in the H616 SoC, it is
>   	  optional for the H6.
> +
> +config SUN55I_PCK600
> +	bool "Allwinner A523 PCK-600 power domain driver"

Any particular reason this is not tristate? The driver advertises itself 
as a platform driver module?

> +	depends on PM
> +	select PM_GENERIC_DOMAINS
> +	help
> +	  Say y to enable the PCK-600 power domain driver. This saves power
> +	  when certain peripherals, such as the video engine, are idle.

If I understand correctly, this driver is *required* to make use of 
those peripherals, and the video engine is not even the most prominent 
user. So regardless of the reset state of the power domain, I think the 
wording should be changed, to make sure distributions activate this 
option. At the moment it sounds highly optional. I wonder if we should 
use "default y if ARCH_SUNXI" even.

> diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
> index c1343e123759..e344b232fc9f 100644
> --- a/drivers/pmdomain/sunxi/Makefile
> +++ b/drivers/pmdomain/sunxi/Makefile
> @@ -1,3 +1,4 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   obj-$(CONFIG_SUN20I_PPU)		+= sun20i-ppu.o
>   obj-$(CONFIG_SUN50I_H6_PRCM_PPU)	+= sun50i-h6-prcm-ppu.o
> +obj-$(CONFIG_SUN55I_PCK600)		+= sun55i-pck600.o
> diff --git a/drivers/pmdomain/sunxi/sun55i-pck600.c b/drivers/pmdomain/sunxi/sun55i-pck600.c
> new file mode 100644
> index 000000000000..7248f6113665
> --- /dev/null
> +++ b/drivers/pmdomain/sunxi/sun55i-pck600.c
> @@ -0,0 +1,225 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Allwinner PCK-600 power domain support

Can you please mention here that this device is based on the Arm PCK-600 
IP, as done in the commit message. And say that this is a minimal 
implementaton, just supporting the off/on states.

Maybe also mention the relevant documentation: the "ARM CoreLink PCK‑600 
Power Control Kit" TRM and the "Arm Power Policy Unit" architecture 
specification (DEN0051E).

> + *
> + * Copyright (c) 2025 Chen-Yu Tsai <wens@csie.org>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/container_of.h>
> +#include <linux/device.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +#include <linux/string_choices.h>
> +
> +#define PPU_PWPR    0x0
> +#define PPU_PWSR    0x8
> +#define	PPU_DCDR0   0x170

white space issue?

> +#define PPU_DCDR1   0x174
> +
> +#define PPU_PWSR_PWR_STATUS	GENMASK(3, 0)

Would just PPU_PWR_STATUS be a better name, since it's used by both the 
PWPR and PWSR registers?

> +#define PPU_POWER_MODE_ON	0x8
> +#define PPU_POWER_MODE_OFF	0x0
> +
> +#define PPU_REG_SIZE	0x1000
> +
> +struct sunxi_pck600_desc {
> +	const char * const *pd_names;
> +	unsigned int num_domains;
> +	u32 logic_power_switch0_delay_offset;
> +	u32 logic_power_switch1_delay_offset;
> +	u32 off2on_delay_offset;
> +	u32 device_ctrl0_delay;
> +	u32 device_ctrl1_delay;
> +	u32 logic_power_switch0_delay;
> +	u32 logic_power_switch1_delay;
> +	u32 off2on_delay;

Is there any indication that those parameters are different between 
different SoCs? I appreciate the idea of making this future-proof, but 
this might be a bit premature, if all SoCs use the same values?

> +};
> +
> +struct sunxi_pck600_pd {
> +	struct generic_pm_domain genpd;
> +	struct sunxi_pck600 *pck;
> +	void __iomem *base;
> +};
> +
> +struct sunxi_pck600 {
> +	struct device *dev;
> +	struct genpd_onecell_data genpd_data;
> +	struct sunxi_pck600_pd pds[];
> +};
> +
> +#define to_sunxi_pd(gpd) container_of(gpd, struct sunxi_pck600_pd, genpd)
> +
> +static int sunxi_pck600_pd_set_power(struct sunxi_pck600_pd *pd, bool on)
> +{
> +	struct sunxi_pck600 *pck = pd->pck;
> +	struct generic_pm_domain *genpd = &pd->genpd;
> +	int ret;
> +	u32 val, reg;
> +
> +	val = on ? PPU_POWER_MODE_ON : PPU_POWER_MODE_OFF;
> +
> +	reg = readl(pd->base + PPU_PWPR);
> +	FIELD_MODIFY(PPU_PWSR_PWR_STATUS, &reg, val);
> +	writel(reg, pd->base + PPU_PWPR);

Don't we need a lock here, or is this covered by the power domain framework?

Cheers,
Andre

> +
> +	/* push write out to hardware */
> +	reg = readl(pd->base + PPU_PWPR);
> +
> +	ret = readl_poll_timeout_atomic(pd->base + PPU_PWSR, reg,
> +					FIELD_GET(PPU_PWSR_PWR_STATUS, reg) == val,
> +					0, 10000);
> +	if (ret)
> +		dev_err(pck->dev, "failed to turn domain \"%s\" %s: %d\n",
> +			genpd->name, str_on_off(on), ret);
> +
> +	return ret;
> +}
> +
> +static int sunxi_pck600_power_on(struct generic_pm_domain *domain)
> +{
> +	struct sunxi_pck600_pd *pd = to_sunxi_pd(domain);
> +
> +	return sunxi_pck600_pd_set_power(pd, true);
> +}
> +
> +static int sunxi_pck600_power_off(struct generic_pm_domain *domain)
> +{
> +	struct sunxi_pck600_pd *pd = to_sunxi_pd(domain);
> +
> +	return sunxi_pck600_pd_set_power(pd, false);
> +}
> +
> +static void sunxi_pck600_pd_setup(struct sunxi_pck600_pd *pd,
> +				  const struct sunxi_pck600_desc *desc)
> +{
> +	writel(desc->device_ctrl0_delay, pd->base + PPU_DCDR0);
> +	writel(desc->device_ctrl1_delay, pd->base + PPU_DCDR1);
> +	writel(desc->logic_power_switch0_delay,
> +	       pd->base + desc->logic_power_switch0_delay_offset);
> +	writel(desc->logic_power_switch1_delay,
> +	       pd->base + desc->logic_power_switch1_delay_offset);
> +	writel(desc->off2on_delay, pd->base + desc->off2on_delay_offset);
> +}
> +
> +static int sunxi_pck600_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const struct sunxi_pck600_desc *desc;
> +	struct genpd_onecell_data *genpds;
> +	struct sunxi_pck600 *pck;
> +	struct reset_control *rst;
> +	struct clk *clk;
> +	void __iomem *base;
> +	int i, ret;
> +
> +	desc = of_device_get_match_data(dev);
> +
> +	pck = devm_kzalloc(dev, struct_size(pck, pds, desc->num_domains), GFP_KERNEL);
> +	if (!pck)
> +		return -ENOMEM;
> +
> +	pck->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, pck);
> +
> +	genpds = &pck->genpd_data;
> +	genpds->num_domains = desc->num_domains;
> +	genpds->domains = devm_kcalloc(dev, desc->num_domains,
> +				       sizeof(*genpds->domains), GFP_KERNEL);
> +	if (!genpds->domains)
> +		return -ENOMEM;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	rst = devm_reset_control_get_exclusive_released(dev, NULL);
> +	if (IS_ERR(rst))
> +		return dev_err_probe(dev, PTR_ERR(rst), "failed to get reset control\n");
> +
> +	clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n");
> +
> +	for (i = 0; i < desc->num_domains; i++) {
> +		struct sunxi_pck600_pd *pd = &pck->pds[i];
> +
> +		pd->genpd.name = desc->pd_names[i];
> +		pd->genpd.power_off = sunxi_pck600_power_off;
> +		pd->genpd.power_on = sunxi_pck600_power_on;
> +		pd->base = base + PPU_REG_SIZE * i;
> +
> +		sunxi_pck600_pd_setup(pd, desc);
> +		ret = pm_genpd_init(&pd->genpd, NULL, false);
> +		if (ret) {
> +			dev_err_probe(dev, ret, "failed to initialize power domain\n");
> +			goto err_remove_pds;
> +		}
> +
> +		genpds->domains[i] = &pd->genpd;
> +	}
> +
> +	ret = of_genpd_add_provider_onecell(dev_of_node(dev), genpds);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to add PD provider\n");
> +		goto err_remove_pds;
> +	}
> +
> +	return 0;
> +
> +err_remove_pds:
> +	for (i--; i >= 0; i--)
> +		pm_genpd_remove(genpds->domains[i]);
> +
> +	return ret;
> +}
> +
> +static const char * const sun55i_a523_pck600_pd_names[] = {
> +	"VE", "GPU", "VI", "VO0", "VO1", "DE", "NAND", "PCIE"
> +};
> +
> +static const struct sunxi_pck600_desc sun55i_a523_pck600_desc = {
> +	.pd_names = sun55i_a523_pck600_pd_names,
> +	.num_domains = ARRAY_SIZE(sun55i_a523_pck600_pd_names),
> +	.logic_power_switch0_delay_offset = 0xc00,
> +	.logic_power_switch1_delay_offset = 0xc04,
> +	.off2on_delay_offset = 0xc10,
> +	.device_ctrl0_delay = 0xffffff,
> +	.device_ctrl1_delay = 0xffff,
> +	.logic_power_switch0_delay = 0x8080808,
> +	.logic_power_switch1_delay = 0x808,
> +	.off2on_delay = 0x8
> +};
> +
> +static const struct of_device_id sunxi_pck600_of_match[] = {
> +	{
> +		.compatible	= "allwinner,sun55i-a523-pck-600",
> +		.data		= &sun55i_a523_pck600_desc,
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, sunxi_pck600_of_match);
> +
> +static struct platform_driver sunxi_pck600_driver = {
> +	.probe = sunxi_pck600_probe,
> +	.driver = {
> +		.name   = "sunxi-pck-600",
> +		.of_match_table = sunxi_pck600_of_match,
> +		/* Power domains cannot be removed if in use. */
> +		.suppress_bind_attrs = true,
> +	},
> +};
> +module_platform_driver(sunxi_pck600_driver);
> +
> +MODULE_DESCRIPTION("Allwinner PCK-600 power domain driver");
> +MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
> +MODULE_LICENSE("GPL");

Re: [PATCH v2 3/4] pmdomain: sunxi: add driver for Allwinner A523's PCK-600 power controller

[Chen-Yu Tsai]

On Thu, Jul 10, 2025 at 7:46 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> Hi Chen-Yu,
>
> thanks for posting this! This is a quick first view, haven't compared
> against the BSP bits yet....
>
> On 09/07/2025 16:53, Chen-Yu Tsai wrote:
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > Allwinner A523 family has a second power controller, named PCK-600 in
> > the datasheets and BSP. It is likely based on ARM's PCK-600 hardware
> > block, with some additional delay controls. The only documentation for
> > this hardware is the BSP driver. The standard registers defined in ARM's
> > Power Policy Unit Architecture Specification line up. Some extra delay
> > controls are found in the reserved range of registers.
> >
> > Add a driver for this power controller. Delay control register values
> > and power domain names are from the BSP driver.
> >
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >   drivers/pmdomain/sunxi/Kconfig         |   8 +
> >   drivers/pmdomain/sunxi/Makefile        |   1 +
> >   drivers/pmdomain/sunxi/sun55i-pck600.c | 225 +++++++++++++++++++++++++
> >   3 files changed, 234 insertions(+)
> >   create mode 100644 drivers/pmdomain/sunxi/sun55i-pck600.c
> >
> > diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
> > index 43eecb3ea981..3e2b77cd9a2b 100644
> > --- a/drivers/pmdomain/sunxi/Kconfig
> > +++ b/drivers/pmdomain/sunxi/Kconfig
> > @@ -18,3 +18,11 @@ config SUN50I_H6_PRCM_PPU
> >         Say y to enable the Allwinner H6/H616 PRCM power domain driver.
> >         This is required to enable the Mali GPU in the H616 SoC, it is
> >         optional for the H6.
> > +
> > +config SUN55I_PCK600
> > +     bool "Allwinner A523 PCK-600 power domain driver"
>
> Any particular reason this is not tristate? The driver advertises itself
> as a platform driver module?

Cargo-culted from the D1 PPU driver. So, no particular reason.

> > +     depends on PM
> > +     select PM_GENERIC_DOMAINS
> > +     help
> > +       Say y to enable the PCK-600 power domain driver. This saves power
> > +       when certain peripherals, such as the video engine, are idle.
>
> If I understand correctly, this driver is *required* to make use of
> those peripherals, and the video engine is not even the most prominent
> user. So regardless of the reset state of the power domain, I think the
> wording should be changed, to make sure distributions activate this
> option. At the moment it sounds highly optional. I wonder if we should
> use "default y if ARCH_SUNXI" even.

Makes sense. Though this was also cargo-culted from the D1 PPU. So I
guess I should fix both.

> > diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
> > index c1343e123759..e344b232fc9f 100644
> > --- a/drivers/pmdomain/sunxi/Makefile
> > +++ b/drivers/pmdomain/sunxi/Makefile
> > @@ -1,3 +1,4 @@
> >   # SPDX-License-Identifier: GPL-2.0-only
> >   obj-$(CONFIG_SUN20I_PPU)            += sun20i-ppu.o
> >   obj-$(CONFIG_SUN50I_H6_PRCM_PPU)    += sun50i-h6-prcm-ppu.o
> > +obj-$(CONFIG_SUN55I_PCK600)          += sun55i-pck600.o
> > diff --git a/drivers/pmdomain/sunxi/sun55i-pck600.c b/drivers/pmdomain/sunxi/sun55i-pck600.c
> > new file mode 100644
> > index 000000000000..7248f6113665
> > --- /dev/null
> > +++ b/drivers/pmdomain/sunxi/sun55i-pck600.c
> > @@ -0,0 +1,225 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Allwinner PCK-600 power domain support
>
> Can you please mention here that this device is based on the Arm PCK-600
> IP, as done in the commit message. And say that this is a minimal
> implementaton, just supporting the off/on states.
>
> Maybe also mention the relevant documentation: the "ARM CoreLink PCK‑600
> Power Control Kit" TRM and the "Arm Power Policy Unit" architecture
> specification (DEN0051E).

Will do.

> > + *
> > + * Copyright (c) 2025 Chen-Yu Tsai <wens@csie.org>
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/container_of.h>
> > +#include <linux/device.h>
> > +#include <linux/dev_printk.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/reset.h>
> > +#include <linux/slab.h>
> > +#include <linux/string_choices.h>
> > +
> > +#define PPU_PWPR    0x0
> > +#define PPU_PWSR    0x8
> > +#define      PPU_DCDR0   0x170
>
> white space issue?
>
> > +#define PPU_DCDR1   0x174
> > +
> > +#define PPU_PWSR_PWR_STATUS  GENMASK(3, 0)
>
> Would just PPU_PWR_STATUS be a better name, since it's used by both the
> PWPR and PWSR registers?
>
> > +#define PPU_POWER_MODE_ON    0x8
> > +#define PPU_POWER_MODE_OFF   0x0
> > +
> > +#define PPU_REG_SIZE 0x1000
> > +
> > +struct sunxi_pck600_desc {
> > +     const char * const *pd_names;
> > +     unsigned int num_domains;
> > +     u32 logic_power_switch0_delay_offset;
> > +     u32 logic_power_switch1_delay_offset;
> > +     u32 off2on_delay_offset;
> > +     u32 device_ctrl0_delay;
> > +     u32 device_ctrl1_delay;
> > +     u32 logic_power_switch0_delay;
> > +     u32 logic_power_switch1_delay;
> > +     u32 off2on_delay;
>
> Is there any indication that those parameters are different between
> different SoCs? I appreciate the idea of making this future-proof, but
> this might be a bit premature, if all SoCs use the same values?

It's hard to tell since the BSP driver only covers the A523. I'd just
keep this the way it is, since it makes it easier to generalize this
to cover PCK-600 in other platforms, if such a need ever presents itself.

> > +};
> > +
> > +struct sunxi_pck600_pd {
> > +     struct generic_pm_domain genpd;
> > +     struct sunxi_pck600 *pck;
> > +     void __iomem *base;
> > +};
> > +
> > +struct sunxi_pck600 {
> > +     struct device *dev;
> > +     struct genpd_onecell_data genpd_data;
> > +     struct sunxi_pck600_pd pds[];
> > +};
> > +
> > +#define to_sunxi_pd(gpd) container_of(gpd, struct sunxi_pck600_pd, genpd)
> > +
> > +static int sunxi_pck600_pd_set_power(struct sunxi_pck600_pd *pd, bool on)
> > +{
> > +     struct sunxi_pck600 *pck = pd->pck;
> > +     struct generic_pm_domain *genpd = &pd->genpd;
> > +     int ret;
> > +     u32 val, reg;
> > +
> > +     val = on ? PPU_POWER_MODE_ON : PPU_POWER_MODE_OFF;
> > +
> > +     reg = readl(pd->base + PPU_PWPR);
> > +     FIELD_MODIFY(PPU_PWSR_PWR_STATUS, &reg, val);
> > +     writel(reg, pd->base + PPU_PWPR);
>
> Don't we need a lock here, or is this covered by the power domain framework?

AFAICT genpd has a lock for each power domain. Since each power domain has
its own set of registers, I think we're good here.


Thanks
ChenYu

> Cheers,
> Andre
>
> > +
> > +     /* push write out to hardware */
> > +     reg = readl(pd->base + PPU_PWPR);
> > +
> > +     ret = readl_poll_timeout_atomic(pd->base + PPU_PWSR, reg,
> > +                                     FIELD_GET(PPU_PWSR_PWR_STATUS, reg) == val,
> > +                                     0, 10000);
> > +     if (ret)
> > +             dev_err(pck->dev, "failed to turn domain \"%s\" %s: %d\n",
> > +                     genpd->name, str_on_off(on), ret);
> > +
> > +     return ret;
> > +}
> > +
> > +static int sunxi_pck600_power_on(struct generic_pm_domain *domain)
> > +{
> > +     struct sunxi_pck600_pd *pd = to_sunxi_pd(domain);
> > +
> > +     return sunxi_pck600_pd_set_power(pd, true);
> > +}
> > +
> > +static int sunxi_pck600_power_off(struct generic_pm_domain *domain)
> > +{
> > +     struct sunxi_pck600_pd *pd = to_sunxi_pd(domain);
> > +
> > +     return sunxi_pck600_pd_set_power(pd, false);
> > +}
> > +
> > +static void sunxi_pck600_pd_setup(struct sunxi_pck600_pd *pd,
> > +                               const struct sunxi_pck600_desc *desc)
> > +{
> > +     writel(desc->device_ctrl0_delay, pd->base + PPU_DCDR0);
> > +     writel(desc->device_ctrl1_delay, pd->base + PPU_DCDR1);
> > +     writel(desc->logic_power_switch0_delay,
> > +            pd->base + desc->logic_power_switch0_delay_offset);
> > +     writel(desc->logic_power_switch1_delay,
> > +            pd->base + desc->logic_power_switch1_delay_offset);
> > +     writel(desc->off2on_delay, pd->base + desc->off2on_delay_offset);
> > +}
> > +
> > +static int sunxi_pck600_probe(struct platform_device *pdev)
> > +{
> > +     struct device *dev = &pdev->dev;
> > +     const struct sunxi_pck600_desc *desc;
> > +     struct genpd_onecell_data *genpds;
> > +     struct sunxi_pck600 *pck;
> > +     struct reset_control *rst;
> > +     struct clk *clk;
> > +     void __iomem *base;
> > +     int i, ret;
> > +
> > +     desc = of_device_get_match_data(dev);
> > +
> > +     pck = devm_kzalloc(dev, struct_size(pck, pds, desc->num_domains), GFP_KERNEL);
> > +     if (!pck)
> > +             return -ENOMEM;
> > +
> > +     pck->dev = &pdev->dev;
> > +     platform_set_drvdata(pdev, pck);
> > +
> > +     genpds = &pck->genpd_data;
> > +     genpds->num_domains = desc->num_domains;
> > +     genpds->domains = devm_kcalloc(dev, desc->num_domains,
> > +                                    sizeof(*genpds->domains), GFP_KERNEL);
> > +     if (!genpds->domains)
> > +             return -ENOMEM;
> > +
> > +     base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(base))
> > +             return PTR_ERR(base);
> > +
> > +     rst = devm_reset_control_get_exclusive_released(dev, NULL);
> > +     if (IS_ERR(rst))
> > +             return dev_err_probe(dev, PTR_ERR(rst), "failed to get reset control\n");
> > +
> > +     clk = devm_clk_get_enabled(dev, NULL);
> > +     if (IS_ERR(clk))
> > +             return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n");
> > +
> > +     for (i = 0; i < desc->num_domains; i++) {
> > +             struct sunxi_pck600_pd *pd = &pck->pds[i];
> > +
> > +             pd->genpd.name = desc->pd_names[i];
> > +             pd->genpd.power_off = sunxi_pck600_power_off;
> > +             pd->genpd.power_on = sunxi_pck600_power_on;
> > +             pd->base = base + PPU_REG_SIZE * i;
> > +
> > +             sunxi_pck600_pd_setup(pd, desc);
> > +             ret = pm_genpd_init(&pd->genpd, NULL, false);
> > +             if (ret) {
> > +                     dev_err_probe(dev, ret, "failed to initialize power domain\n");
> > +                     goto err_remove_pds;
> > +             }
> > +
> > +             genpds->domains[i] = &pd->genpd;
> > +     }
> > +
> > +     ret = of_genpd_add_provider_onecell(dev_of_node(dev), genpds);
> > +     if (ret) {
> > +             dev_err_probe(dev, ret, "failed to add PD provider\n");
> > +             goto err_remove_pds;
> > +     }
> > +
> > +     return 0;
> > +
> > +err_remove_pds:
> > +     for (i--; i >= 0; i--)
> > +             pm_genpd_remove(genpds->domains[i]);
> > +
> > +     return ret;
> > +}
> > +
> > +static const char * const sun55i_a523_pck600_pd_names[] = {
> > +     "VE", "GPU", "VI", "VO0", "VO1", "DE", "NAND", "PCIE"
> > +};
> > +
> > +static const struct sunxi_pck600_desc sun55i_a523_pck600_desc = {
> > +     .pd_names = sun55i_a523_pck600_pd_names,
> > +     .num_domains = ARRAY_SIZE(sun55i_a523_pck600_pd_names),
> > +     .logic_power_switch0_delay_offset = 0xc00,
> > +     .logic_power_switch1_delay_offset = 0xc04,
> > +     .off2on_delay_offset = 0xc10,
> > +     .device_ctrl0_delay = 0xffffff,
> > +     .device_ctrl1_delay = 0xffff,
> > +     .logic_power_switch0_delay = 0x8080808,
> > +     .logic_power_switch1_delay = 0x808,
> > +     .off2on_delay = 0x8
> > +};
> > +
> > +static const struct of_device_id sunxi_pck600_of_match[] = {
> > +     {
> > +             .compatible     = "allwinner,sun55i-a523-pck-600",
> > +             .data           = &sun55i_a523_pck600_desc,
> > +     },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(of, sunxi_pck600_of_match);
> > +
> > +static struct platform_driver sunxi_pck600_driver = {
> > +     .probe = sunxi_pck600_probe,
> > +     .driver = {
> > +             .name   = "sunxi-pck-600",
> > +             .of_match_table = sunxi_pck600_of_match,
> > +             /* Power domains cannot be removed if in use. */
> > +             .suppress_bind_attrs = true,
> > +     },
> > +};
> > +module_platform_driver(sunxi_pck600_driver);
> > +
> > +MODULE_DESCRIPTION("Allwinner PCK-600 power domain driver");
> > +MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
> > +MODULE_LICENSE("GPL");
>
>

Re: [PATCH v2 1/4] dt-bindings: power: Add A523 PPU and PCK600 power controllers

[Rob Herring (Arm)]

On Wed, 09 Jul 2025 23:53:40 +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The A523 PPU is likely the same kind of hardware seen on previous SoCs.
> 
> The A523 PCK600, as the name suggests, is likely a customized version
> of ARM's PCK-600 power controller. Comparing the BSP driver against
> ARM's PPU datasheet shows that the basic registers line up, but
> Allwinner's hardware has some additional delay controls in the reserved
> register range. As such it is likely not fully compatible with the
> standard ARM version.
> 
> Document A523 PPU and PCK600 compatibles.
> 
> Also reorder the compatible string entries so they are grouped and
> ordered by family first, then by SoC model.
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> 
> ---
> Changes since v1:
> - Re-order compatible string entries
> - Fix name of header file to match compatible string
> ---
>  .../bindings/power/allwinner,sun20i-d1-ppu.yaml   |  4 +++-
>  .../power/allwinner,sun55i-a523-pck-600.h         | 15 +++++++++++++++
>  .../dt-bindings/power/allwinner,sun55i-a523-ppu.h | 12 ++++++++++++
>  3 files changed, 30 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/power/allwinner,sun55i-a523-pck-600.h
>  create mode 100644 include/dt-bindings/power/allwinner,sun55i-a523-ppu.h
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 3/4] pmdomain: sunxi: add driver for Allwinner A523's PCK-600 power controller

[Andre Przywara]

Hi,

On 10/07/2025 05:14, Chen-Yu Tsai wrote:
> On Thu, Jul 10, 2025 at 7:46 AM Andre Przywara <andre.przywara@arm.com> wrote:
>>
>> Hi Chen-Yu,
>>
>> thanks for posting this! This is a quick first view, haven't compared
>> against the BSP bits yet....
>>
>> On 09/07/2025 16:53, Chen-Yu Tsai wrote:
>>> From: Chen-Yu Tsai <wens@csie.org>
>>>
>>> Allwinner A523 family has a second power controller, named PCK-600 in
>>> the datasheets and BSP. It is likely based on ARM's PCK-600 hardware
>>> block, with some additional delay controls. The only documentation for
>>> this hardware is the BSP driver. The standard registers defined in ARM's
>>> Power Policy Unit Architecture Specification line up. Some extra delay
>>> controls are found in the reserved range of registers.
>>>
>>> Add a driver for this power controller. Delay control register values
>>> and power domain names are from the BSP driver.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>    drivers/pmdomain/sunxi/Kconfig         |   8 +
>>>    drivers/pmdomain/sunxi/Makefile        |   1 +
>>>    drivers/pmdomain/sunxi/sun55i-pck600.c | 225 +++++++++++++++++++++++++
>>>    3 files changed, 234 insertions(+)
>>>    create mode 100644 drivers/pmdomain/sunxi/sun55i-pck600.c
>>>
>>> diff --git a/drivers/pmdomain/sunxi/Kconfig b/drivers/pmdomain/sunxi/Kconfig
>>> index 43eecb3ea981..3e2b77cd9a2b 100644
>>> --- a/drivers/pmdomain/sunxi/Kconfig
>>> +++ b/drivers/pmdomain/sunxi/Kconfig
>>> @@ -18,3 +18,11 @@ config SUN50I_H6_PRCM_PPU
>>>          Say y to enable the Allwinner H6/H616 PRCM power domain driver.
>>>          This is required to enable the Mali GPU in the H616 SoC, it is
>>>          optional for the H6.
>>> +
>>> +config SUN55I_PCK600
>>> +     bool "Allwinner A523 PCK-600 power domain driver"
>>
>> Any particular reason this is not tristate? The driver advertises itself
>> as a platform driver module?
> 
> Cargo-culted from the D1 PPU driver. So, no particular reason.
> 
>>> +     depends on PM
>>> +     select PM_GENERIC_DOMAINS
>>> +     help
>>> +       Say y to enable the PCK-600 power domain driver. This saves power
>>> +       when certain peripherals, such as the video engine, are idle.
>>
>> If I understand correctly, this driver is *required* to make use of
>> those peripherals, and the video engine is not even the most prominent
>> user. So regardless of the reset state of the power domain, I think the
>> wording should be changed, to make sure distributions activate this
>> option. At the moment it sounds highly optional. I wonder if we should
>> use "default y if ARCH_SUNXI" even.
> 
> Makes sense. Though this was also cargo-culted from the D1 PPU. So I
> guess I should fix both.
> 
>>> diff --git a/drivers/pmdomain/sunxi/Makefile b/drivers/pmdomain/sunxi/Makefile
>>> index c1343e123759..e344b232fc9f 100644
>>> --- a/drivers/pmdomain/sunxi/Makefile
>>> +++ b/drivers/pmdomain/sunxi/Makefile
>>> @@ -1,3 +1,4 @@
>>>    # SPDX-License-Identifier: GPL-2.0-only
>>>    obj-$(CONFIG_SUN20I_PPU)            += sun20i-ppu.o
>>>    obj-$(CONFIG_SUN50I_H6_PRCM_PPU)    += sun50i-h6-prcm-ppu.o
>>> +obj-$(CONFIG_SUN55I_PCK600)          += sun55i-pck600.o
>>> diff --git a/drivers/pmdomain/sunxi/sun55i-pck600.c b/drivers/pmdomain/sunxi/sun55i-pck600.c
>>> new file mode 100644
>>> index 000000000000..7248f6113665
>>> --- /dev/null
>>> +++ b/drivers/pmdomain/sunxi/sun55i-pck600.c
>>> @@ -0,0 +1,225 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Allwinner PCK-600 power domain support
>>
>> Can you please mention here that this device is based on the Arm PCK-600
>> IP, as done in the commit message. And say that this is a minimal
>> implementaton, just supporting the off/on states.
>>
>> Maybe also mention the relevant documentation: the "ARM CoreLink PCK‑600
>> Power Control Kit" TRM and the "Arm Power Policy Unit" architecture
>> specification (DEN0051E).
> 
> Will do.
> 
>>> + *
>>> + * Copyright (c) 2025 Chen-Yu Tsai <wens@csie.org>
>>> + */
>>> +
>>> +#include <linux/bitfield.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/container_of.h>
>>> +#include <linux/device.h>
>>> +#include <linux/dev_printk.h>
>>> +#include <linux/err.h>
>>> +#include <linux/io.h>
>>> +#include <linux/iopoll.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_domain.h>
>>> +#include <linux/reset.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/string_choices.h>
>>> +
>>> +#define PPU_PWPR    0x0
>>> +#define PPU_PWSR    0x8
>>> +#define      PPU_DCDR0   0x170
>>
>> white space issue?
>>
>>> +#define PPU_DCDR1   0x174
>>> +
>>> +#define PPU_PWSR_PWR_STATUS  GENMASK(3, 0)
>>
>> Would just PPU_PWR_STATUS be a better name, since it's used by both the
>> PWPR and PWSR registers?
>>
>>> +#define PPU_POWER_MODE_ON    0x8
>>> +#define PPU_POWER_MODE_OFF   0x0
>>> +
>>> +#define PPU_REG_SIZE 0x1000
>>> +
>>> +struct sunxi_pck600_desc {
>>> +     const char * const *pd_names;
>>> +     unsigned int num_domains;
>>> +     u32 logic_power_switch0_delay_offset;
>>> +     u32 logic_power_switch1_delay_offset;
>>> +     u32 off2on_delay_offset;
>>> +     u32 device_ctrl0_delay;
>>> +     u32 device_ctrl1_delay;
>>> +     u32 logic_power_switch0_delay;
>>> +     u32 logic_power_switch1_delay;
>>> +     u32 off2on_delay;
>>
>> Is there any indication that those parameters are different between
>> different SoCs? I appreciate the idea of making this future-proof, but
>> this might be a bit premature, if all SoCs use the same values?
> 
> It's hard to tell since the BSP driver only covers the A523. I'd just
> keep this the way it is, since it makes it easier to generalize this
> to cover PCK-600 in other platforms, if such a need ever presents itself.

Yeah, fair enough, I was just curious. It's fine either way, we can 
adjust this internal detail later, if need be.

>>> +};
>>> +
>>> +struct sunxi_pck600_pd {
>>> +     struct generic_pm_domain genpd;
>>> +     struct sunxi_pck600 *pck;
>>> +     void __iomem *base;
>>> +};
>>> +
>>> +struct sunxi_pck600 {
>>> +     struct device *dev;
>>> +     struct genpd_onecell_data genpd_data;
>>> +     struct sunxi_pck600_pd pds[];
>>> +};
>>> +
>>> +#define to_sunxi_pd(gpd) container_of(gpd, struct sunxi_pck600_pd, genpd)
>>> +
>>> +static int sunxi_pck600_pd_set_power(struct sunxi_pck600_pd *pd, bool on)
>>> +{
>>> +     struct sunxi_pck600 *pck = pd->pck;
>>> +     struct generic_pm_domain *genpd = &pd->genpd;
>>> +     int ret;
>>> +     u32 val, reg;
>>> +
>>> +     val = on ? PPU_POWER_MODE_ON : PPU_POWER_MODE_OFF;
>>> +
>>> +     reg = readl(pd->base + PPU_PWPR);
>>> +     FIELD_MODIFY(PPU_PWSR_PWR_STATUS, &reg, val);
>>> +     writel(reg, pd->base + PPU_PWPR);
>>
>> Don't we need a lock here, or is this covered by the power domain framework?
> 
> AFAICT genpd has a lock for each power domain. Since each power domain has
> its own set of registers, I think we're good here.

Right, I was adittedly lazy yesterday, but now looked it up: indeed 
there is a lock around every call to genpd_power_{off,on}, so it's all good.

Cheers,
Andre


> 
> 
> Thanks
> ChenYu
> 
>> Cheers,
>> Andre
>>
>>> +
>>> +     /* push write out to hardware */
>>> +     reg = readl(pd->base + PPU_PWPR);
>>> +
>>> +     ret = readl_poll_timeout_atomic(pd->base + PPU_PWSR, reg,
>>> +                                     FIELD_GET(PPU_PWSR_PWR_STATUS, reg) == val,
>>> +                                     0, 10000);
>>> +     if (ret)
>>> +             dev_err(pck->dev, "failed to turn domain \"%s\" %s: %d\n",
>>> +                     genpd->name, str_on_off(on), ret);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static int sunxi_pck600_power_on(struct generic_pm_domain *domain)
>>> +{
>>> +     struct sunxi_pck600_pd *pd = to_sunxi_pd(domain);
>>> +
>>> +     return sunxi_pck600_pd_set_power(pd, true);
>>> +}
>>> +
>>> +static int sunxi_pck600_power_off(struct generic_pm_domain *domain)
>>> +{
>>> +     struct sunxi_pck600_pd *pd = to_sunxi_pd(domain);
>>> +
>>> +     return sunxi_pck600_pd_set_power(pd, false);
>>> +}
>>> +
>>> +static void sunxi_pck600_pd_setup(struct sunxi_pck600_pd *pd,
>>> +                               const struct sunxi_pck600_desc *desc)
>>> +{
>>> +     writel(desc->device_ctrl0_delay, pd->base + PPU_DCDR0);
>>> +     writel(desc->device_ctrl1_delay, pd->base + PPU_DCDR1);
>>> +     writel(desc->logic_power_switch0_delay,
>>> +            pd->base + desc->logic_power_switch0_delay_offset);
>>> +     writel(desc->logic_power_switch1_delay,
>>> +            pd->base + desc->logic_power_switch1_delay_offset);
>>> +     writel(desc->off2on_delay, pd->base + desc->off2on_delay_offset);
>>> +}
>>> +
>>> +static int sunxi_pck600_probe(struct platform_device *pdev)
>>> +{
>>> +     struct device *dev = &pdev->dev;
>>> +     const struct sunxi_pck600_desc *desc;
>>> +     struct genpd_onecell_data *genpds;
>>> +     struct sunxi_pck600 *pck;
>>> +     struct reset_control *rst;
>>> +     struct clk *clk;
>>> +     void __iomem *base;
>>> +     int i, ret;
>>> +
>>> +     desc = of_device_get_match_data(dev);
>>> +
>>> +     pck = devm_kzalloc(dev, struct_size(pck, pds, desc->num_domains), GFP_KERNEL);
>>> +     if (!pck)
>>> +             return -ENOMEM;
>>> +
>>> +     pck->dev = &pdev->dev;
>>> +     platform_set_drvdata(pdev, pck);
>>> +
>>> +     genpds = &pck->genpd_data;
>>> +     genpds->num_domains = desc->num_domains;
>>> +     genpds->domains = devm_kcalloc(dev, desc->num_domains,
>>> +                                    sizeof(*genpds->domains), GFP_KERNEL);
>>> +     if (!genpds->domains)
>>> +             return -ENOMEM;
>>> +
>>> +     base = devm_platform_ioremap_resource(pdev, 0);
>>> +     if (IS_ERR(base))
>>> +             return PTR_ERR(base);
>>> +
>>> +     rst = devm_reset_control_get_exclusive_released(dev, NULL);
>>> +     if (IS_ERR(rst))
>>> +             return dev_err_probe(dev, PTR_ERR(rst), "failed to get reset control\n");
>>> +
>>> +     clk = devm_clk_get_enabled(dev, NULL);
>>> +     if (IS_ERR(clk))
>>> +             return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n");
>>> +
>>> +     for (i = 0; i < desc->num_domains; i++) {
>>> +             struct sunxi_pck600_pd *pd = &pck->pds[i];
>>> +
>>> +             pd->genpd.name = desc->pd_names[i];
>>> +             pd->genpd.power_off = sunxi_pck600_power_off;
>>> +             pd->genpd.power_on = sunxi_pck600_power_on;
>>> +             pd->base = base + PPU_REG_SIZE * i;
>>> +
>>> +             sunxi_pck600_pd_setup(pd, desc);
>>> +             ret = pm_genpd_init(&pd->genpd, NULL, false);
>>> +             if (ret) {
>>> +                     dev_err_probe(dev, ret, "failed to initialize power domain\n");
>>> +                     goto err_remove_pds;
>>> +             }
>>> +
>>> +             genpds->domains[i] = &pd->genpd;
>>> +     }
>>> +
>>> +     ret = of_genpd_add_provider_onecell(dev_of_node(dev), genpds);
>>> +     if (ret) {
>>> +             dev_err_probe(dev, ret, "failed to add PD provider\n");
>>> +             goto err_remove_pds;
>>> +     }
>>> +
>>> +     return 0;
>>> +
>>> +err_remove_pds:
>>> +     for (i--; i >= 0; i--)
>>> +             pm_genpd_remove(genpds->domains[i]);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static const char * const sun55i_a523_pck600_pd_names[] = {
>>> +     "VE", "GPU", "VI", "VO0", "VO1", "DE", "NAND", "PCIE"
>>> +};
>>> +
>>> +static const struct sunxi_pck600_desc sun55i_a523_pck600_desc = {
>>> +     .pd_names = sun55i_a523_pck600_pd_names,
>>> +     .num_domains = ARRAY_SIZE(sun55i_a523_pck600_pd_names),
>>> +     .logic_power_switch0_delay_offset = 0xc00,
>>> +     .logic_power_switch1_delay_offset = 0xc04,
>>> +     .off2on_delay_offset = 0xc10,
>>> +     .device_ctrl0_delay = 0xffffff,
>>> +     .device_ctrl1_delay = 0xffff,
>>> +     .logic_power_switch0_delay = 0x8080808,
>>> +     .logic_power_switch1_delay = 0x808,
>>> +     .off2on_delay = 0x8
>>> +};
>>> +
>>> +static const struct of_device_id sunxi_pck600_of_match[] = {
>>> +     {
>>> +             .compatible     = "allwinner,sun55i-a523-pck-600",
>>> +             .data           = &sun55i_a523_pck600_desc,
>>> +     },
>>> +     {}
>>> +};
>>> +MODULE_DEVICE_TABLE(of, sunxi_pck600_of_match);
>>> +
>>> +static struct platform_driver sunxi_pck600_driver = {
>>> +     .probe = sunxi_pck600_probe,
>>> +     .driver = {
>>> +             .name   = "sunxi-pck-600",
>>> +             .of_match_table = sunxi_pck600_of_match,
>>> +             /* Power domains cannot be removed if in use. */
>>> +             .suppress_bind_attrs = true,
>>> +     },
>>> +};
>>> +module_platform_driver(sunxi_pck600_driver);
>>> +
>>> +MODULE_DESCRIPTION("Allwinner PCK-600 power domain driver");
>>> +MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
>>> +MODULE_LICENSE("GPL");
>>
>>
>