Re: [PATCH v3 2/2] pmdomain: rockchip: Add RK3572 boot peripheral domains

[Finley Xiao <finley.xiao@rock-chips.com>]

在 2026/10/26 11:21, Cunhao Lu via B4 Relay 写道:
> From: Finley Xiao <finley.xiao@rock-chips.com>
>
> Add support for the RK3572 NVM0, NVM1, PHP, and VOP power domains.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
>
> - Returned firmware shift-ready wait errors to callers.
> - Kept the domain pending after a firmware shift-ready wait failure.
> - Retried power-on without repeating completed memory resets.
> - Recorded low-level power-on and restoration stages before hardware
>    operations so retries complete unfinished low-level work.
> - Kept memory resets pending across failures, including when memory is off.
> - Preserved QoS snapshots after a failed power-on and cleared progress
>    after successful power-on or shutdown.
> - Tracked incomplete shutdowns to preserve QoS and complete firmware updates.
> - Preserved pending memory resets across shutdown and cancelled unfinished
>    power-on requests.
> - Attempt to restore the domain after power-off errors, while preserving
>    the original error returned to genpd.
>
> Co-developed-by: Cunhao Lu <1579567540@qq.com>
> Signed-off-by: Cunhao Lu <1579567540@qq.com>
> ---
> Changes in v3:
> - Track incomplete shutdowns to preserve QoS and complete firmware updates.
> - Preserve pending memory resets across shutdown and cancel unfinished
>    power-on requests.
> - Attempt to restore the domain after power-off errors, while preserving
>    the original error returned to genpd.
>
> Changes in v2:
> - Record low-level power-on and restoration stages before hardware
>    operations, so a retry cannot skip unfinished low-level work.
> - Retain pending memory resets even if memory is off on retry, and
>    avoid repeating completed resets during restoration.
> - Preserve QoS snapshots across failed power-on and shutdown; clear
>    progress after successful power-on or shutdown.
> ---
>   drivers/pmdomain/rockchip/pm-domains.c | 187 +++++++++++++++++++++++++++++----
>   1 file changed, 167 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index e193f978d5f3..668d208dea55 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -39,6 +39,7 @@
>   #include <dt-bindings/power/rockchip,rk3528-power.h>
>   #include <dt-bindings/power/rockchip,rk3562-power.h>
>   #include <dt-bindings/power/rk3568-power.h>
> +#include <dt-bindings/power/rockchip,rk3572-power-controller.h>
>   #include <dt-bindings/power/rockchip,rk3576-power.h>
>   #include <dt-bindings/power/rk3588-power.h>
>   
> @@ -59,6 +60,9 @@ struct rockchip_domain_info {
>   	u32 pwr_offset;
>   	u32 mem_offset;
>   	u32 req_offset;
> +	u32 status_offset;
> +	u32 firewall_mask;
> +	u32 clk_ungate_offset;
>   };
>   
>   struct rockchip_pmu_info {
> @@ -72,6 +76,8 @@ struct rockchip_pmu_info {
>   	u32 mem_status_offset;
>   	u32 repair_status_offset;
>   	u32 clk_ungate_offset;
> +	u32 firewall_status_offset;
> +	bool chain_status_inverted;
>   
please keep the abbreviation consistent with the other struct
members and use fw_mask / fw_status_offset instead of firewall_mask /
firewall_status_offset.
>   	u32 core_pwrcnt_offset;
>   	u32 gpu_pwrcnt_offset;
> @@ -90,6 +96,13 @@ struct rockchip_pmu_info {
>   #define QOS_SATURATION		0x14
>   #define QOS_EXTCONTROL		0x18
>   
> +enum rockchip_pd_power_state {
> +	ROCKCHIP_PD_IDLE,
> +	ROCKCHIP_PD_ON_HW,
> +	ROCKCHIP_PD_ON_RESTORE,
> +	ROCKCHIP_PD_OFF,
> +};
> +
>   struct rockchip_pm_domain {
>   	struct generic_pm_domain genpd;
>   	const struct rockchip_domain_info *info;
> @@ -101,11 +114,14 @@ struct rockchip_pm_domain {
>   	struct clk_bulk_data *clks;
>   	struct device_node *node;
>   	struct regulator *supply;
> +	enum rockchip_pd_power_state power_state;
> +	bool mem_reset_pending;
>   };
>   
>   struct rockchip_pmu {
>   	struct device *dev;
>   	struct regmap *regmap;
> +	struct regmap *grf;
>   	const struct rockchip_pmu_info *info;
>   	struct mutex mutex; /* mutex lock for pmu */
>   	struct genpd_onecell_data genpd_data;
> @@ -373,15 +389,16 @@ static bool rockchip_pmu_domain_is_idle(struct rockchip_pm_domain *pd)
>   	const struct rockchip_domain_info *pd_info = pd->info;
>   	unsigned int val;
>   
> -	regmap_read(pmu->regmap, pmu->info->idle_offset, &val);
> +	regmap_read(pmu->regmap, pmu->info->idle_offset + pd_info->status_offset, &val);
>   	return (val & pd_info->idle_mask) == pd_info->idle_mask;
>   }
>   
> -static unsigned int rockchip_pmu_read_ack(struct rockchip_pmu *pmu)
> +static unsigned int rockchip_pmu_read_ack(struct rockchip_pm_domain *pd)
>   {
> +	struct rockchip_pmu *pmu = pd->pmu;
>   	unsigned int val;
>   
> -	regmap_read(pmu->regmap, pmu->info->ack_offset, &val);
> +	regmap_read(pmu->regmap, pmu->info->ack_offset + pd->info->status_offset, &val);
>   	return val;
>   }
>   
> @@ -400,7 +417,8 @@ static int rockchip_pmu_ungate_clk(struct rockchip_pm_domain *pd, bool ungate)
>   
>   	val = ungate ? (pd_info->clk_ungate_mask | clk_ungate_w_mask) :
>   			clk_ungate_w_mask;
> -	regmap_write(pmu->regmap, pmu->info->clk_ungate_offset, val);
> +	regmap_write(pmu->regmap, pmu->info->clk_ungate_offset +
> +		     pd_info->clk_ungate_offset, val);
>   
>   	return 0;
>   }
> @@ -431,7 +449,7 @@ static int rockchip_pmu_set_idle_request(struct rockchip_pm_domain *pd,
>   
>   	/* Wait util idle_ack = 1 */
>   	target_ack = idle ? pd_info->ack_mask : 0;
> -	ret = readx_poll_timeout_atomic(rockchip_pmu_read_ack, pmu, val,
> +	ret = readx_poll_timeout_atomic(rockchip_pmu_read_ack, pd, val,
>   					(val & pd_info->ack_mask) == target_ack,
>   					0, 10000);
>   	if (ret) {
> @@ -543,6 +561,9 @@ static bool rockchip_pmu_domain_is_chain_on(struct rockchip_pm_domain *pd)
>   	regmap_read(pmu->regmap,
>   		    pmu->info->chain_status_offset + pd->info->mem_offset, &val);
>   
> +	if (pmu->info->chain_status_inverted)
> +		return !(val & pd->info->mem_status_mask);
> +
>   	/* 1'b1: power on, 1'b0: power off */
>   	return val & pd->info->mem_status_mask;
>   }
> @@ -600,16 +621,13 @@ static int rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
>   	struct rockchip_pmu *pmu = pd->pmu;
>   	struct generic_pm_domain *genpd = &pd->genpd;
>   	u32 pd_pwr_offset = pd->info->pwr_offset;
> -	bool is_on, is_mem_on = false;
> +	bool is_on;
>   	struct arm_smccc_res res;
>   	int ret;
>   
>   	if (pd->info->pwr_mask == 0)
>   		return 0;
>   
> -	if (on && pd->info->mem_status_mask)
> -		is_mem_on = rockchip_pmu_domain_is_mem_on(pd);
> -
>   	if (pd->info->pwr_w_mask)
>   		regmap_write(pmu->regmap, pmu->info->pwr_offset + pd_pwr_offset,
>   			     on ? pd->info->pwr_w_mask :
> @@ -620,13 +638,13 @@ static int rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
>   
>   	wmb();
>   
> -	if (is_mem_on) {
> +	if (on && pd->mem_reset_pending) {
>   		ret = rockchip_pmu_domain_mem_reset(pd);
>   		if (ret)
>   			return ret;
> +		pd->mem_reset_pending = false;
>   	}
>   
> -
>   	ret = readx_poll_timeout_atomic(rockchip_pmu_domain_is_on, pd, is_on,
>   					is_on == on, 0, 10000);
>   	if (ret) {
> @@ -644,14 +662,29 @@ static int rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
>   	return 0;
>   }
>   
> +static int rockchip_pmu_wait_firewall(struct rockchip_pm_domain *pd)
> +{
> +	struct rockchip_pmu *pmu = pd->pmu;
> +	u32 mask = pd->info->firewall_mask;
> +	u32 val;
> +
> +	if (!mask)
> +		return 0;
> +
> +	return regmap_read_poll_timeout(pmu->grf, pmu->info->firewall_status_offset,
> +				       val, (val & mask) == mask, 0, 10000);
> +}
> +
>   static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>   {
>   	struct rockchip_pmu *pmu = pd->pmu;
> +	bool is_on;
>   	int ret;
>   
>   	guard(mutex)(&pmu->mutex);
>   
> -	if (rockchip_pmu_domain_is_on(pd) == power_on)
> +	is_on = rockchip_pmu_domain_is_on(pd);
> +	if (is_on == power_on && pd->power_state == ROCKCHIP_PD_IDLE)
>   		return 0;
>   
>   	ret = clk_bulk_enable(pd->num_clks, pd->clks);
> @@ -662,18 +695,38 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>   
>   	rockchip_pmu_ungate_clk(pd, true);
>   
> -	if (!power_on) {
> -		rockchip_pmu_save_qos(pd);
> +	if (power_on) {
> +		if (pd->power_state == ROCKCHIP_PD_IDLE ||
> +		    pd->power_state == ROCKCHIP_PD_OFF ||
> +		    (pd->power_state == ROCKCHIP_PD_ON_RESTORE && !is_on)) {
> +			/* A failed reset may still be forcing memory off. */
> +			pd->mem_reset_pending |= pd->info->mem_status_mask &&
> +				rockchip_pmu_domain_is_mem_on(pd);
> +			pd->power_state = ROCKCHIP_PD_ON_HW;
> +		}
> +
> +		/* A status bit alone does not prove the low-level sequence finished. */
> +		if (pd->power_state == ROCKCHIP_PD_ON_HW) {
> +			ret = rockchip_do_pmu_set_power_domain(pd, true);
> +			if (ret < 0)
> +				goto out;
> +			pd->power_state = ROCKCHIP_PD_ON_RESTORE;
> +		}
> +	} else {
> +		if (pd->power_state == ROCKCHIP_PD_IDLE)
> +			rockchip_pmu_save_qos(pd);
> +		pd->power_state = ROCKCHIP_PD_OFF;
>   
>   		/* if powering down, idle request to NIU first */
>   		ret = rockchip_pmu_set_idle_request(pd, true);
>   		if (ret < 0)
>   			goto out;
> -	}
>   
> -	ret = rockchip_do_pmu_set_power_domain(pd, power_on);
> -	if (ret < 0)
> -		goto out;
> +		/* Cancel a pending power-on even if its status is still off. */
> +		ret = rockchip_do_pmu_set_power_domain(pd, false);
> +		if (ret < 0)
> +			goto out;
> +	}
>   
>   	if (power_on) {
>   		/* if powering up, leave idle mode */
> @@ -681,8 +734,14 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>   		if (ret < 0)
>   			goto out;
>   
> +		ret = rockchip_pmu_wait_firewall(pd);
> +		if (ret)
> +			goto out;
> +
>   		rockchip_pmu_restore_qos(pd);
> +		pd->mem_reset_pending = false;
>   	}
> +	pd->power_state = ROCKCHIP_PD_IDLE;
>   
>   out:
>   	rockchip_pmu_ungate_clk(pd, false);
> @@ -733,11 +792,18 @@ static int rockchip_pd_power_on(struct generic_pm_domain *domain)
>   static int rockchip_pd_power_off(struct generic_pm_domain *domain)
>   {
>   	struct rockchip_pm_domain *pd = to_rockchip_pd(domain);
> -	int ret;
> +	int ret, err;
>   
>   	ret = rockchip_pd_power(pd, false);
> -	if (ret)
> +	if (ret) {
> +		/* genpd still considers the domain on after a failed power-off. */
> +		err = rockchip_pd_power(pd, true);
> +		if (err)
> +			dev_err(pd->pmu->dev,
> +				"failed to restore domain '%s' after power-off failure: %d\n",
> +				domain->name, err);
>   		return ret;
> +	}

The changes to rockchip_pd_power()/rockchip_pd_power_off() (the
power_state state machine, mem_reset_pending, and the restore after
a failed power-off) are generic rework of the shared power path;
they are not required for basic RK3572 support and they affect all
the other SoCs. Please split them out of this series.


>   
>   	rockchip_pd_regulator_disable(pd);
>   	return ret;
> @@ -1064,6 +1130,11 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
>   	mutex_init(&pmu->mutex);
>   
>   	pmu->info = pmu_info;
> +	if (pmu_info->firewall_status_offset) {
> +		pmu->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> +		if (IS_ERR(pmu->grf))
> +			return dev_err_probe(dev, PTR_ERR(pmu->grf), "failed to get system GRF\n");
> +	}
>   
>   	pmu->genpd_data.domains = pmu->domains;
>   	pmu->genpd_data.num_domains = pmu_info->num_domains;
> @@ -1313,6 +1384,62 @@ static const struct rockchip_domain_info rk3568_pm_domains[] = {
>   	[RK3568_PD_PIPE]	= DOMAIN_RK3568("pipe", BIT(8), BIT(11), false, false),
>   };
>   
> +static const struct rockchip_domain_info rk3572_pm_domains[] = {
> +	[RK3572_PD_NVM1] = {
> +		.name = "nvm1",
> +		.pwr_mask = BIT(4),
> +		.pwr_w_mask = BIT(20),
> +		.mem_status_mask = BIT(4),
> +		.repair_status_mask = BIT(3),
> +		.req_mask = BIT(6),
> +		.req_w_mask = BIT(22),
> +		.idle_mask = BIT(6),
> +		.ack_mask = BIT(6),
> +		.clk_ungate_mask = BIT(6),
> +	},
> +	[RK3572_PD_PHP] = {
> +		.name = "php",
> +		.pwr_mask = BIT(7),
> +		.pwr_w_mask = BIT(23),
> +		.mem_status_mask = BIT(7),
> +		.repair_status_mask = BIT(5),
> +		.req_mask = BIT(9),
> +		.req_w_mask = BIT(25),
> +		.idle_mask = BIT(9),
> +		.ack_mask = BIT(9),
> +		.clk_ungate_mask = BIT(9),
> +		.firewall_mask = BIT(28),
> +	},
> +	[RK3572_PD_VOP] = {
> +		.name = "vop",
> +		.pwr_mask = BIT(12),
> +		.pwr_w_mask = BIT(28),
> +		.mem_status_mask = BIT(12),
> +		.repair_status_mask = BIT(10),
> +		.req_mask = BIT(14),
> +		.req_w_mask = BIT(30),
> +		.idle_mask = BIT(14),
> +		.ack_mask = BIT(14),
> +		.clk_ungate_mask = BIT(14),
> +		.firewall_mask = BIT(23),
> +	},
> +	[RK3572_PD_NVM0] = {
> +		.name = "nvm0",
> +		.pwr_mask = BIT(15),
> +		.pwr_w_mask = BIT(31),
> +		.mem_status_mask = BIT(15),
> +		.repair_status_mask = BIT(13),
> +		.req_mask = BIT(0),
> +		.req_w_mask = BIT(16),
> +		.idle_mask = BIT(0),
> +		.ack_mask = BIT(0),
> +		.clk_ungate_mask = BIT(0),
> +		.req_offset = 4,
> +		.status_offset = 4,
> +		.clk_ungate_offset = 4,
> +	},
> +};
1. Only 4 of the 14 domains are added here. The internal implementation
    covers npu, gpu, nvm0, nvm1, dsmc, php, audio, img, vdec, vi, venc,
    vo, vop and vop_pre. Please add the remaining domains.

    Note that the npu domain also needs the voltage-domain handshake from
    the internal code (vd_offset = 0x220, per-domain vd_en and a
    rockchip_pmu_vd_on() call before powering on the domain), so either
    include that support as well, or leave out npu until it is ready.

2. Please use a DOMAIN_RK3572 macro like the other SoCs in this
    driver (see DOMAIN_RK3576).
> +
>   static const struct rockchip_domain_info rk3576_pm_domains[] = {
>   	[RK3576_PD_NPU]		= DOMAIN_RK3576("npu",    0x0, BIT(0),  BIT(0), 0,       0x0, 0,       0,       0,       false),
>   	[RK3576_PD_NVM]		= DOMAIN_RK3576("nvm",    0x0, BIT(6),  0,      BIT(6),  0x4, BIT(2),  BIT(18), BIT(2),  false),
> @@ -1536,6 +1663,23 @@ static const struct rockchip_pmu_info rk3568_pmu = {
>   	.domain_info = rk3568_pm_domains,
>   };
>   
> +static const struct rockchip_pmu_info rk3572_pmu = {
> +	.pwr_offset = 0x210,
> +	.status_offset = 0x230,
> +	.chain_status_offset = 0x250,
> +	.chain_status_inverted = true,
> +	.mem_status_offset = 0x260,
> +	.mem_pwr_offset = 0x300,
> +	.req_offset = 0x110,
> +	.idle_offset = 0x130,
> +	.ack_offset = 0x120,
> +	.repair_status_offset = 0x570,
> +	.clk_ungate_offset = 0x150,
> +	.firewall_status_offset = 0x140,
> +	.num_domains = ARRAY_SIZE(rk3572_pm_domains),
> +	.domain_info = rk3572_pm_domains,
> +};
> +
>   static const struct rockchip_pmu_info rk3576_pmu = {
>   	.pwr_offset = 0x210,
>   	.status_offset = 0x230,
> @@ -1648,6 +1792,9 @@ static const struct of_device_id rockchip_pm_domain_dt_match[] = {
>   		.data = (void *)&rk3568_pmu,
>   	},
>   	{
> +		.compatible = "rockchip,rk3572-power-controller",
> +		.data = &rk3572_pmu,
> +	}, {
>   		.compatible = "rockchip,rk3576-power-controller",
>   		.data = (void *)&rk3576_pmu,
>   	},
>
-- 
Best regards,

底层平台中心 肖锋（Finley Xiao）
************************************************************************************
瑞芯微电子股份有限公司
Rockchip Electronics Co., Ltd.
福建省福州市铜盘路软件大道89号软件园A区21号楼 350003
No.21 Building, A District, Fuzhou Software Park, Fuzhou, Fujian 350003, P.R. China
Tel: 0591-83991906-8602 Mobile: 18506057603
************************************************************************************