Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver

[Iker Pedrosa]

El vie, 18 sept 2026 a las 7:49, Yixun Lan (<dlan@xxxxxxxxxx>) escribió:
> [...]
> diff --git a/drivers/pmdomain/spacemit/pm_domains.c b/drivers/pmdomain/spacemit/pm_domains.c
> new file mode 100644
> index 000000000000..d563e4e4e232
> --- /dev/null
> +++ b/drivers/pmdomain/spacemit/pm_domains.c
> [...]
> +static struct spacemit_pmu *gpmu;

Would it make sense to embed the 'struct spacemit_pmu' pointer directly into
'struct spacemit_pm_domain'? Other generic Power Domain drivers (Rockchip,
QCOM, Renesas) use this pattern to keep domain callbacks self-contained and
ready for multi-instance SoCs

> [...]
> +static int spacemit_pd_power_on(struct generic_pm_domain *domain)
> +{
> [...]
> + regmap_read(gpmu->regmap, APMU_POWER_STATUS_REG, &val);

Please check the return value of regmap_read()

> [...]
> + if (ret < 0) {
> + dev_err(&domain->dev, "power-off domain: %d, error\n", spd->pm_index);

Typo: should this say "power-on domain" since this is inside
spacemit_pd_power_on()?

> [...]
> +static bool spacemit_pm_get_state(struct spacemit_pmu *pmu,
> +  struct spacemit_pm_domain *pd)
> +{
> + const struct spacemit_pm_domain_param *p = pd->param;
> + u32 reg, bit;
> +
> + regmap_read(pmu->regmap, APMU_POWER_STATUS_REG, &reg);

Please check the return value here as well

> + bit = p->use_hw ? BIT(pd->param->bit_hw_pwr_stat) :
> +  BIT(pd->param->bit_pwr_stat);

Since 'p' is initialized to 'pd->param' above, you can use 'p->bit_hw_pwr_stat'
and 'p->bit_pwr_stat' directly here