Hi Yixun,

On Sun, 08 Nov 2026 10:36:02 +0000, Yixun Lan wrote:
> +config SPACEMIT_PM_DOMAINS
> +	tristate "PM Domain driver for SpacemiT SoC"
[...]
> +	  Say y to enable device power management through PM domains.
This is tristate now, so "Say Y or M".

[...]
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/regmap.h>
> +#include <linux/auxiliary_bus.h>
io.h, err.h and platform_device.h are not used right now.

[...]
> +static int spacemit_pd_power_off(struct generic_pm_domain *domain)
[...]
> +	if (ret) {
> +		dev_err(&domain->dev, "failed to power off domain %s: %d\n",
> +			p->name, pd->pm_index);
> +		return -EBUSY;
> +	}

-EBUSY drops the -ETIMEDOUT returned by regmap_read_poll_timeout();
why not print ret and return it? Same in spacemit_pd_power_on().

[...]
> +static bool spacemit_pm_is_off_state(struct spacemit_pmu *pmu,
> +				     struct spacemit_pm_domain *pd)
[...]
> +	return (reg & bit) ? false : true;
return !(reg & bit);

[...]
> +static void spacemit_pm_domain_cleanup(struct spacemit_pmu *pmu)
> +{
> +	struct spacemit_pm_domain *pd;
> +	int i, ret;
> +
> +	for (i = 0; i < pmu->genpd_data.num_domains; i++) {
> +		if (!pmu->domains[i])
> +			continue;
> +
> +		pd = to_spacemit_pd(pmu->domains[i]);
> +		ret = pm_genpd_remove(pmu->domains[i]);
> +		if (ret)
> +			dev_err(pmu->dev, "failed to remove domain %s: %d - state may be inconsistent\n",
> +				pd->param->name, ret);
> +	}
> +}
[...]
> +static void spacemit_pm_domain_remove(struct auxiliary_device *adev)
> +{
> +	struct spacemit_pmu *pmu = auxiliary_get_drvdata(adev);
> +	struct device *dev = &adev->dev;
> +
> +	of_genpd_del_provider(dev->of_node);
> +	spacemit_pm_domain_cleanup(pmu);
> +}
pm_genpd_remove() fails with -EBUSY while a consumer is still attached,
but pd is freed by devres anyway. Set suppress_bind_attrs like most
pmdomain drivers?

-- 
Troy Mitchell