* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-18 2:40 ` [PATCH 2/3] pmdomain: spacemit: Add power domain driver Yixun Lan
@ 2026-09-18 9:00 ` Icenowy Zheng
2026-09-18 23:01 ` Yixun Lan
2026-09-20 12:56 ` Icenowy Zheng
2026-10-02 18:54 ` Iker Pedrosa
2 siblings, 1 reply; 21+ messages in thread
From: Icenowy Zheng @ 2026-09-18 9:00 UTC (permalink / raw)
To: Yixun Lan, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Ulf Hansson, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti
Cc: devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> SpacemiT's PMU (Power Management Unit) consist of several power
> domains
> which can be managed independently, depending on different
> application
> scenario, each domain can be powered on/off for saving power.
>
> The driver is implemented based on Linux Generic PM Domain
> framework.
> From a hardware perspective, either of two distinct power-on
> sequences
> are supported, in software mode (SW mode), the driver is responsible
> for
> controlling the states of bits such as sleep1, sleep2, isolation and
> pwr_state, while in hardware mode (HW mode), the PMU hardware will
> complete
> the sequence automatically without requiring software intervention.
I think there also exists solutions for K3 that uses RPMI power
domains, and let their ESOS firmware to access the hardware.
What's the relationship of this implementation with the ESOS
implementation? Should the ESOS part be disabled to use this
implementation?
Thanks,
Icenowy
>
> Signed-off-by: Yixun Lan <dlan@kernel.org>
> ---
> drivers/pmdomain/Kconfig | 1 +
> drivers/pmdomain/Makefile | 1 +
> drivers/pmdomain/spacemit/Kconfig | 15 ++
> drivers/pmdomain/spacemit/Makefile | 1 +
> drivers/pmdomain/spacemit/pm_domains.c | 448
> +++++++++++++++++++++++++++++++++
> 5 files changed, 466 insertions(+)
>
> diff --git a/drivers/pmdomain/Kconfig b/drivers/pmdomain/Kconfig
> index 23076ae90e66..0610acd030f6 100644
> --- a/drivers/pmdomain/Kconfig
> +++ b/drivers/pmdomain/Kconfig
> @@ -13,6 +13,7 @@ source "drivers/pmdomain/qcom/Kconfig"
> source "drivers/pmdomain/renesas/Kconfig"
> source "drivers/pmdomain/rockchip/Kconfig"
> source "drivers/pmdomain/samsung/Kconfig"
> +source "drivers/pmdomain/spacemit/Kconfig"
> source "drivers/pmdomain/st/Kconfig"
> source "drivers/pmdomain/starfive/Kconfig"
> source "drivers/pmdomain/sunxi/Kconfig"
> diff --git a/drivers/pmdomain/Makefile b/drivers/pmdomain/Makefile
> index ebc802f13eb9..ba25b444e263 100644
> --- a/drivers/pmdomain/Makefile
> +++ b/drivers/pmdomain/Makefile
> @@ -11,6 +11,7 @@ obj-y += qcom/
> obj-y += renesas/
> obj-y += rockchip/
> obj-y += samsung/
> +obj-y += spacemit/
> obj-y += st/
> obj-y += starfive/
> obj-y += sunxi/
> diff --git a/drivers/pmdomain/spacemit/Kconfig
> b/drivers/pmdomain/spacemit/Kconfig
> new file mode 100644
> index 000000000000..4b927beedc7c
> --- /dev/null
> +++ b/drivers/pmdomain/spacemit/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +menu "SpacemiT PM Domains"
> +
> +config SPACEMIT_PM_DOMAINS
> + bool "PM Domain driver for SpacemiT SoC"
> + depends on PM
> + depends on OF
> + depends on MFD_SYSCON
> + select PM_GENERIC_DOMAINS
> + help
> + PM Domain driver for SpacemiT SoC. Say Y if you want to
> support
> + SpacemiT SoCs.
> +
> +endmenu
> diff --git a/drivers/pmdomain/spacemit/Makefile
> b/drivers/pmdomain/spacemit/Makefile
> new file mode 100644
> index 000000000000..5e8a740ba494
> --- /dev/null
> +++ b/drivers/pmdomain/spacemit/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_SPACEMIT_PM_DOMAINS) += pm_domains.o
> 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
> @@ -0,0 +1,448 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Spacemit Generic power domain support.
> + *
> + * Copyright (c) 2026 SpacemiT Technology Co. Ltd
> + */
> +
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <dt-bindings/power/spacemit,k1-power.h>
> +#include <dt-bindings/power/spacemit,k3-power.h>
> +
> +#define APMU_POWER_STATUS_REG 0xf0
> +#define APMU_POWER_TIMEOUT_US 10000
> +
> +struct spacemit_pm_domain_param {
> + int reg_pwr_ctrl;
> + int bit_hw_mode;
> + int bit_sleep2;
> + int bit_sleep1;
> + int bit_isolation;
> + int bit_auto_pwr_on;
> + int bit_hw_pwr_stat;
> + int bit_pwr_stat;
> + int use_hw;
> + const char *name;
> +};
> +
> +struct spacemit_pm_domain {
> + struct generic_pm_domain genpd;
> + int pm_index;
> + const struct spacemit_pm_domain_param *param;
> +};
> +
> +struct spacemit_pmu {
> + struct device *dev;
> + struct genpd_onecell_data genpd_data;
> + struct regmap *regmap;
> + struct spacemit_pm_domain **domains;
> + int num_domains;
> +};
> +
> +struct spacemit_pm_of_data {
> + int num_domains;
> + const struct spacemit_pm_domain_param *param;
> +};
> +
> +static struct spacemit_pmu *gpmu;
> +
> +static int spacemit_pd_power_off(struct generic_pm_domain *domain)
> +{
> + struct spacemit_pm_domain *spd = container_of(domain, struct
> spacemit_pm_domain, genpd);
> + const struct spacemit_pm_domain_param *p = spd->param;
> + unsigned int val;
> + int ret;
> +
> + if (!spd->param->use_hw) {
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> BIT(p->bit_isolation));
> + fsleep(15);
> +
> + val = BIT(p->bit_sleep1) | BIT(p->bit_sleep2);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(15);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + !(val & BIT(p-
> >bit_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + } else {
> + val = BIT(p->bit_auto_pwr_on) | BIT(p->bit_hw_mode);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(15);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + !(val & BIT(p-
> >bit_hw_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + }
> +
> + if (ret) {
> + dev_err(&domain->dev, "Fail to power-off domain:
> %d\n",
> + spd->pm_index);
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
> +static int spacemit_pd_power_on(struct generic_pm_domain *domain)
> +{
> + struct spacemit_pm_domain *spd = container_of(domain, struct
> spacemit_pm_domain, genpd);
> + const struct spacemit_pm_domain_param *p = spd->param;
> + unsigned int val;
> + int ret = 0;
> +
> + regmap_read(gpmu->regmap, APMU_POWER_STATUS_REG, &val);
> +
> + if (!p->use_hw && (val & BIT(p->bit_pwr_stat))) {
> + val = BIT(p->bit_isolation);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(15);
> +
> + val = BIT(p->bit_sleep1) | BIT(p->bit_sleep2);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(15);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + !(val & BIT(p-
> >bit_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + }
> +
> + if (p->use_hw && (val & BIT(p->bit_hw_pwr_stat))) {
> + val = BIT(p->bit_auto_pwr_on) | BIT(p->bit_hw_mode);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(30);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + !(val & BIT(p-
> >bit_hw_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + }
> +
> + if (ret < 0) {
> + dev_err(&domain->dev, "power-off domain: %d,
> error\n", spd->pm_index);
> + return -EBUSY;
> + }
> +
> + if (!p->use_hw) {
> + regmap_set_bits(gpmu->regmap, p->reg_pwr_ctrl,
> BIT(p->bit_sleep1));
> + fsleep(20);
> +
> + val = BIT(p->bit_sleep2) | BIT(p->bit_sleep1);
> + regmap_set_bits(gpmu->regmap, p->reg_pwr_ctrl, val);
> + fsleep(20);
> +
> + regmap_set_bits(gpmu->regmap, p->reg_pwr_ctrl,
> BIT(p->bit_isolation));
> + fsleep(15);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + (val & BIT(p-
> >bit_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + } else {
> + val = BIT(p->bit_auto_pwr_on) | BIT(p->bit_hw_mode);
> + regmap_set_bits(gpmu->regmap, p->reg_pwr_ctrl, val);
> + fsleep(300);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + (val & BIT(p-
> >bit_hw_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + }
> +
> + if (ret < 0) {
> + dev_err(&domain->dev, "power-on domain: %d,
> error\n", spd->pm_index);
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
> +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, ®);
> +
> + bit = p->use_hw ? BIT(pd->param->bit_hw_pwr_stat) :
> + BIT(pd->param->bit_pwr_stat);
> +
> + return !!(reg & bit);
> +}
> +
> +static int spacemit_pm_add_one_domain(struct spacemit_pmu *pmu, int
> id,
> + const struct
> spacemit_pm_domain_param *param)
> +{
> + struct spacemit_pm_domain *pd;
> +
> + pd = devm_kzalloc(pmu->dev, sizeof(*pd), GFP_KERNEL);
> + if (!pd)
> + return -ENOMEM;
> +
> + pd->pm_index = id;
> + pd->param = param;
> + pd->genpd.name = param->name;
> + pd->genpd.power_off = spacemit_pd_power_off;
> + pd->genpd.power_on = spacemit_pd_power_on;
> +
> + pm_genpd_init(&pd->genpd, NULL, !spacemit_pm_get_state(pmu,
> pd));
> +
> + pmu->domains[id] = pd;
> +
> + return 0;
> +}
> +
> +static void spacemit_pm_domain_cleanup(struct spacemit_pmu *pmu)
> +{
> + int i;
> +
> + for (i = 0; i < pmu->num_domains; i++) {
> + if (pmu->domains[i])
> + pm_genpd_remove(&pmu->domains[i]->genpd);
> + }
> +}
> +
> +static int spacemit_pm_domain_probe(struct platform_device *pdev)
> +{
> + const struct spacemit_pm_of_data *data;
> + struct device *dev = &pdev->dev;
> + struct spacemit_pmu *pmu;
> + int err, i;
> +
> + data = device_get_match_data(dev);
> +
> + pmu = devm_kzalloc(dev, sizeof(*pmu), GFP_KERNEL);
> + if (!pmu)
> + return -ENOMEM;
> +
> + pmu->dev = dev;
> + pmu->num_domains = data->num_domains;
> +
> + pmu->regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
> "spacemit,apmu");
> + if (IS_ERR(pmu->regmap))
> + return dev_err_probe(dev, PTR_ERR(pmu->regmap),
> "failed to get apmu regmap\n");
> +
> + pmu->domains = devm_kcalloc(dev, data->num_domains,
> + sizeof(*pmu->domains),
> GFP_KERNEL);
> + if (!pmu->domains)
> + return -ENOMEM;
> +
> + for (i = 0; i < data->num_domains; i++) {
> + err = spacemit_pm_add_one_domain(pmu, i, &data-
> >param[i]);
> + if (err) {
> + dev_err(dev, "failed to add domain %d:
> %d\n", i, err);
> + goto err_out;
> + }
> + }
> +
> + pmu->genpd_data.domains = (struct generic_pm_domain
> **)pmu->domains;
> + pmu->genpd_data.num_domains = data->num_domains;
> +
> + err = of_genpd_add_provider_onecell(dev->of_node, &pmu-
> >genpd_data);
> + if (err) {
> + dev_err(dev, "failed to add provider: %d\n", err);
> + goto err_out;
> + }
> +
> + gpmu = pmu;
> +
> + return 0;
> +
> +err_out:
> + spacemit_pm_domain_cleanup(pmu);
> + return err;
> +}
> +
> +static const struct spacemit_pm_domain_param k1_domain_params[] = {
> + [K1_PMDOMAIN_VPU] = {
> + .reg_pwr_ctrl = 0xa8,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_pwr_stat = 1,
> + .bit_hw_pwr_stat = 9,
> + .name = "vpu",
> + },
> + [K1_PMDOMAIN_GPU] = {
> + .reg_pwr_ctrl = 0xd0,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_pwr_stat = 0,
> + .name = "gpu",
> + },
> + [K1_PMDOMAIN_LCD] = {
> + .reg_pwr_ctrl = 0x380,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 4,
> + .bit_hw_pwr_stat = 12,
> + .use_hw = 1,
> + .name = "lcd",
> + },
> + [K1_PMDOMAIN_ISP] = {
> + .reg_pwr_ctrl = 0x37c,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 2,
> + .bit_hw_pwr_stat = 10,
> + .name = "isp",
> + },
> + [K1_PMDOMAIN_AUDIO] = {
> + .reg_pwr_ctrl = 0x378,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 3,
> + .bit_hw_pwr_stat = 11,
> + .use_hw = 1,
> + .name = "audio",
> + },
> + [K1_PMDOMAIN_GNSS] = {
> + .reg_pwr_ctrl = 0x13c,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 6,
> + .bit_hw_pwr_stat = 14,
> + .name = "gnss",
> + },
> + [K1_PMDOMAIN_HDMI] = {
> + .reg_pwr_ctrl = 0x3f4,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 7,
> + .bit_hw_pwr_stat = 15,
> + .use_hw = 1,
> + .name = "hdmi",
> + },
> +};
> +
> +static const struct spacemit_pm_domain_param k3_domain_params[] = {
> + [K3_PMDOMAIN_VPU] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 2,
> + .bit_hw_pwr_stat = 9,
> + .use_hw = 1,
> + .reg_pwr_ctrl = 0xa8,
> + .name = "vpu",
> + },
> + [K3_PMDOMAIN_GPU] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 0,
> + .bit_hw_pwr_stat = 8,
> + .use_hw = 1,
> + .reg_pwr_ctrl = 0xd0,
> + .name = "gpu",
> + },
> + [K3_PMDOMAIN_AUDIO] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 3,
> + .reg_pwr_ctrl = 0x378,
> + .name = "audio",
> + },
> + [K3_PMDOMAIN_LCD0] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 4,
> + .bit_hw_pwr_stat = 12,
> + .use_hw = 1,
> + .reg_pwr_ctrl = 0x380,
> + .name = "lcd0",
> + },
> + [K3_PMDOMAIN_LCD1] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 5,
> + .bit_hw_pwr_stat = 15,
> + .use_hw = 1,
> + .reg_pwr_ctrl = 0x3f4,
> + .name = "lcd1",
> + },
> +};
> +
> +static const struct spacemit_pm_of_data k1_of_data = {
> + .num_domains = ARRAY_SIZE(k1_domain_params),
> + .param = k1_domain_params,
> +};
> +
> +static const struct spacemit_pm_of_data k3_of_data = {
> + .num_domains = ARRAY_SIZE(k3_domain_params),
> + .param = k3_domain_params,
> +};
> +
> +static const struct of_device_id spacemit_pm_domain_dt_match[] = {
> + {
> + .compatible = "spacemit,k1-power-controller",
> + .data = &k1_of_data,
> + },
> + {
> + .compatible = "spacemit,k3-power-controller",
> + .data = &k3_of_data,
> + },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver spacemit_pm_domain_driver = {
> + .probe = spacemit_pm_domain_probe,
> + .driver = {
> + .name = "spacemit-pm-domain",
> + .of_match_table = spacemit_pm_domain_dt_match,
> + },
> +};
> +
> +builtin_platform_driver(spacemit_pm_domain_driver);
> +MODULE_DESCRIPTION("SpacemiT Generic Power Domains driver");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-18 9:00 ` Icenowy Zheng
@ 2026-09-18 23:01 ` Yixun Lan
2026-09-19 6:57 ` Icenowy Zheng
0 siblings, 1 reply; 21+ messages in thread
From: Yixun Lan @ 2026-09-18 23:01 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
Hi Icenowy,
On 17:00 Fri 18 Sep , Icenowy Zheng wrote:
> 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > SpacemiT's PMU (Power Management Unit) consist of several power
> > domains
> > which can be managed independently, depending on different
> > application
> > scenario, each domain can be powered on/off for saving power.
> >
> > The driver is implemented based on Linux Generic PM Domain
> > framework.
> > From a hardware perspective, either of two distinct power-on
> > sequences
> > are supported, in software mode (SW mode), the driver is responsible
> > for
> > controlling the states of bits such as sleep1, sleep2, isolation and
> > pwr_state, while in hardware mode (HW mode), the PMU hardware will
> > complete
> > the sequence automatically without requiring software intervention.
>
> I think there also exists solutions for K3 that uses RPMI power
> domains, and let their ESOS firmware to access the hardware.
>
Yes, I'm aware of that
> What's the relationship of this implementation with the ESOS
> implementation? Should the ESOS part be disabled to use this
> implementation?
>
I should say, it's a mutually exclusive solution, so yes, the ESOS part
should be disabled in order to use this version
The motivation of this patch is trying to support drm/display driver's
PM requirement, while avoid going with vendor's complicated RPMI solution
which isn't mainline ready
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-18 23:01 ` Yixun Lan
@ 2026-09-19 6:57 ` Icenowy Zheng
2026-09-19 9:59 ` Yixun Lan
0 siblings, 1 reply; 21+ messages in thread
From: Icenowy Zheng @ 2026-09-19 6:57 UTC (permalink / raw)
To: Yixun Lan
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
在 2026-09-18五的 23:01 +0000，Yixun Lan写道：
> Hi Icenowy,
>
> On 17:00 Fri 18 Sep , Icenowy Zheng wrote:
> > 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > > SpacemiT's PMU (Power Management Unit) consist of several power
> > > domains
> > > which can be managed independently, depending on different
> > > application
> > > scenario, each domain can be powered on/off for saving power.
> > >
> > > The driver is implemented based on Linux Generic PM Domain
> > > framework.
> > > From a hardware perspective, either of two distinct power-on
> > > sequences
> > > are supported, in software mode (SW mode), the driver is
> > > responsible
> > > for
> > > controlling the states of bits such as sleep1, sleep2, isolation
> > > and
> > > pwr_state, while in hardware mode (HW mode), the PMU hardware
> > > will
> > > complete
> > > the sequence automatically without requiring software
> > > intervention.
> >
> > I think there also exists solutions for K3 that uses RPMI power
> > domains, and let their ESOS firmware to access the hardware.
> >
> Yes, I'm aware of that
>
> > What's the relationship of this implementation with the ESOS
> > implementation? Should the ESOS part be disabled to use this
> > implementation?
> >
> I should say, it's a mutually exclusive solution, so yes, the ESOS
> part
> should be disabled in order to use this version
>
> The motivation of this patch is trying to support drm/display
> driver's
> PM requirement, while avoid going with vendor's complicated RPMI
> solution
> which isn't mainline ready
Is this a long-term solution, or a temporary one?
If it's a temporary conflicting one, I don't think it's worth picking
at all.
Thanks,
Icenowy
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-19 6:57 ` Icenowy Zheng
@ 2026-09-19 9:59 ` Yixun Lan
2026-09-19 13:56 ` Icenowy Zheng
0 siblings, 1 reply; 21+ messages in thread
From: Yixun Lan @ 2026-09-19 9:59 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
Hi Icenowy,
On 14:57 Sat 19 Sep , Icenowy Zheng wrote:
> 在 2026-09-18五的 23:01 +0000，Yixun Lan写道：
> > Hi Icenowy,
> >
> > On 17:00 Fri 18 Sep , Icenowy Zheng wrote:
> > > 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > > > SpacemiT's PMU (Power Management Unit) consist of several power
> > > > domains
> > > > which can be managed independently, depending on different
> > > > application
> > > > scenario, each domain can be powered on/off for saving power.
> > > >
> > > > The driver is implemented based on Linux Generic PM Domain
> > > > framework.
> > > > From a hardware perspective, either of two distinct power-on
> > > > sequences
> > > > are supported, in software mode (SW mode), the driver is
> > > > responsible
> > > > for
> > > > controlling the states of bits such as sleep1, sleep2, isolation
> > > > and
> > > > pwr_state, while in hardware mode (HW mode), the PMU hardware
> > > > will
> > > > complete
> > > > the sequence automatically without requiring software
> > > > intervention.
> > >
> > > I think there also exists solutions for K3 that uses RPMI power
> > > domains, and let their ESOS firmware to access the hardware.
> > >
> > Yes, I'm aware of that
> >
> > > What's the relationship of this implementation with the ESOS
> > > implementation? Should the ESOS part be disabled to use this
> > > implementation?
> > >
> > I should say, it's a mutually exclusive solution, so yes, the ESOS
> > part
> > should be disabled in order to use this version
> >
> > The motivation of this patch is trying to support drm/display
> > driver's
> > PM requirement, while avoid going with vendor's complicated RPMI
> > solution
> > which isn't mainline ready
>
> Is this a long-term solution, or a temporary one?
>
For upstream, I'd say we probably will stick to this solution for
long time, won't say it 'forever', see comment below..
> If it's a temporary conflicting one, I don't think it's worth picking
> at all.
>
Well, checked current Linux kernel status, there is no RPMI PM solution
implemented although there is documentation in riscv-rpmi spec[1]
On the other hand, I see no problem to switch to RPMI solution once it
became mature, it's very similar to two solution of clock CLK_CCU vs CLK_RPMI..
Link: https://github.com/riscv-non-isa/riscv-rpmi/blob/main/src/srvgrp-device-power.adoc [1]
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-19 9:59 ` Yixun Lan
@ 2026-09-19 13:56 ` Icenowy Zheng
2026-09-19 23:13 ` Yixun Lan
2026-09-20 8:00 ` Troy Mitchell
0 siblings, 2 replies; 21+ messages in thread
From: Icenowy Zheng @ 2026-09-19 13:56 UTC (permalink / raw)
To: Yixun Lan
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
在 2026-09-19六的 09:59 +0000，Yixun Lan写道：
> Hi Icenowy,
>
> On 14:57 Sat 19 Sep , Icenowy Zheng wrote:
> > 在 2026-09-18五的 23:01 +0000，Yixun Lan写道：
> > > Hi Icenowy,
> > >
> > > On 17:00 Fri 18 Sep , Icenowy Zheng wrote:
> > > > 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > > > > SpacemiT's PMU (Power Management Unit) consist of several
> > > > > power
> > > > > domains
> > > > > which can be managed independently, depending on different
> > > > > application
> > > > > scenario, each domain can be powered on/off for saving power.
> > > > >
> > > > > The driver is implemented based on Linux Generic PM Domain
> > > > > framework.
> > > > > From a hardware perspective, either of two distinct power-on
> > > > > sequences
> > > > > are supported, in software mode (SW mode), the driver is
> > > > > responsible
> > > > > for
> > > > > controlling the states of bits such as sleep1, sleep2,
> > > > > isolation
> > > > > and
> > > > > pwr_state, while in hardware mode (HW mode), the PMU hardware
> > > > > will
> > > > > complete
> > > > > the sequence automatically without requiring software
> > > > > intervention.
> > > >
> > > > I think there also exists solutions for K3 that uses RPMI power
> > > > domains, and let their ESOS firmware to access the hardware.
> > > >
> > > Yes, I'm aware of that
> > >
> > > > What's the relationship of this implementation with the ESOS
> > > > implementation? Should the ESOS part be disabled to use this
> > > > implementation?
> > > >
> > > I should say, it's a mutually exclusive solution, so yes, the
> > > ESOS
> > > part
> > > should be disabled in order to use this version
> > >
> > > The motivation of this patch is trying to support drm/display
> > > driver's
> > > PM requirement, while avoid going with vendor's complicated RPMI
> > > solution
> > > which isn't mainline ready
> >
> > Is this a long-term solution, or a temporary one?
> >
> For upstream, I'd say we probably will stick to this solution for
> long time, won't say it 'forever', see comment below..
>
> > If it's a temporary conflicting one, I don't think it's worth
> > picking
> > at all.
> >
> Well, checked current Linux kernel status, there is no RPMI PM
> solution
> implemented although there is documentation in riscv-rpmi spec[1]
I have seen pending patch for it with multiple revisions [1], the dt
binding is even already ACKed by Conor, so I think it's not a big
problem here.
Thanks,
Icenowy
[1] 20260903092347.620060-1-joshua.yeong@starfivetech.com
>
> On the other hand, I see no problem to switch to RPMI solution once
> it
> became mature, it's very similar to two solution of clock CLK_CCU vs
> CLK_RPMI..
>
> Link:
> https://github.com/riscv-non-isa/riscv-rpmi/blob/main/src/srvgrp-device-power.adoc
> [1]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-19 13:56 ` Icenowy Zheng
@ 2026-09-19 23:13 ` Yixun Lan
2026-09-20 5:17 ` Icenowy Zheng
2026-09-20 8:00 ` Troy Mitchell
1 sibling, 1 reply; 21+ messages in thread
From: Yixun Lan @ 2026-09-19 23:13 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
Hi Icenowy,
On 21:56 Sat 19 Sep , Icenowy Zheng wrote:
> 在 2026-09-19六的 09:59 +0000，Yixun Lan写道：
> > Hi Icenowy,
> >
> > On 14:57 Sat 19 Sep , Icenowy Zheng wrote:
> > > 在 2026-09-18五的 23:01 +0000，Yixun Lan写道：
> > > > Hi Icenowy,
> > > >
> > > > On 17:00 Fri 18 Sep , Icenowy Zheng wrote:
> > > > > 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > > > > > SpacemiT's PMU (Power Management Unit) consist of several
> > > > > > power
> > > > > > domains
> > > > > > which can be managed independently, depending on different
> > > > > > application
> > > > > > scenario, each domain can be powered on/off for saving power.
> > > > > >
> > > > > > The driver is implemented based on Linux Generic PM Domain
> > > > > > framework.
> > > > > > From a hardware perspective, either of two distinct power-on
> > > > > > sequences
> > > > > > are supported, in software mode (SW mode), the driver is
> > > > > > responsible
> > > > > > for
> > > > > > controlling the states of bits such as sleep1, sleep2,
> > > > > > isolation
> > > > > > and
> > > > > > pwr_state, while in hardware mode (HW mode), the PMU hardware
> > > > > > will
> > > > > > complete
> > > > > > the sequence automatically without requiring software
> > > > > > intervention.
> > > > >
> > > > > I think there also exists solutions for K3 that uses RPMI power
> > > > > domains, and let their ESOS firmware to access the hardware.
> > > > >
> > > > Yes, I'm aware of that
> > > >
> > > > > What's the relationship of this implementation with the ESOS
> > > > > implementation? Should the ESOS part be disabled to use this
> > > > > implementation?
> > > > >
> > > > I should say, it's a mutually exclusive solution, so yes, the
> > > > ESOS
> > > > part
> > > > should be disabled in order to use this version
> > > >
> > > > The motivation of this patch is trying to support drm/display
> > > > driver's
> > > > PM requirement, while avoid going with vendor's complicated RPMI
> > > > solution
> > > > which isn't mainline ready
> > >
> > > Is this a long-term solution, or a temporary one?
> > >
> > For upstream, I'd say we probably will stick to this solution for
> > long time, won't say it 'forever', see comment below..
> >
> > > If it's a temporary conflicting one, I don't think it's worth
> > > picking
> > > at all.
> > >
> > Well, checked current Linux kernel status, there is no RPMI PM
> > solution
> > implemented although there is documentation in riscv-rpmi spec[1]
>
> I have seen pending patch for it with multiple revisions [1], the dt
> binding is even already ACKed by Conor, so I think it's not a big
> problem here.
>
That's exactly what I said - it's not mature, not saying it's pending
patch for review, but for whole solution that involve Linux driver,
opensbi and underlying fimware (ESOS) - which not upstream-ed..
To make it clear, I'm not going to pursue the RPMI solution, but if
someone insist and willing to spend the effort, please feel free to push
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-19 23:13 ` Yixun Lan
@ 2026-09-20 5:17 ` Icenowy Zheng
0 siblings, 0 replies; 21+ messages in thread
From: Icenowy Zheng @ 2026-09-20 5:17 UTC (permalink / raw)
To: Yixun Lan
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
在 2026-09-19六的 23:13 +0000，Yixun Lan写道：
> Hi Icenowy,
>
> On 21:56 Sat 19 Sep , Icenowy Zheng wrote:
> > 在 2026-09-19六的 09:59 +0000，Yixun Lan写道：
> > > Hi Icenowy,
> > >
> > > On 14:57 Sat 19 Sep , Icenowy Zheng wrote:
> > > > 在 2026-09-18五的 23:01 +0000，Yixun Lan写道：
> > > > > Hi Icenowy,
> > > > >
> > > > > On 17:00 Fri 18 Sep , Icenowy Zheng wrote:
> > > > > > 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > > > > > > SpacemiT's PMU (Power Management Unit) consist of
> > > > > > > several
> > > > > > > power
> > > > > > > domains
> > > > > > > which can be managed independently, depending on
> > > > > > > different
> > > > > > > application
> > > > > > > scenario, each domain can be powered on/off for saving
> > > > > > > power.
> > > > > > >
> > > > > > > The driver is implemented based on Linux Generic PM
> > > > > > > Domain
> > > > > > > framework.
> > > > > > > From a hardware perspective, either of two distinct
> > > > > > > power-on
> > > > > > > sequences
> > > > > > > are supported, in software mode (SW mode), the driver is
> > > > > > > responsible
> > > > > > > for
> > > > > > > controlling the states of bits such as sleep1, sleep2,
> > > > > > > isolation
> > > > > > > and
> > > > > > > pwr_state, while in hardware mode (HW mode), the PMU
> > > > > > > hardware
> > > > > > > will
> > > > > > > complete
> > > > > > > the sequence automatically without requiring software
> > > > > > > intervention.
> > > > > >
> > > > > > I think there also exists solutions for K3 that uses RPMI
> > > > > > power
> > > > > > domains, and let their ESOS firmware to access the
> > > > > > hardware.
> > > > > >
> > > > > Yes, I'm aware of that
> > > > >
> > > > > > What's the relationship of this implementation with the
> > > > > > ESOS
> > > > > > implementation? Should the ESOS part be disabled to use
> > > > > > this
> > > > > > implementation?
> > > > > >
> > > > > I should say, it's a mutually exclusive solution, so yes, the
> > > > > ESOS
> > > > > part
> > > > > should be disabled in order to use this version
> > > > >
> > > > > The motivation of this patch is trying to support drm/display
> > > > > driver's
> > > > > PM requirement, while avoid going with vendor's complicated
> > > > > RPMI
> > > > > solution
> > > > > which isn't mainline ready
> > > >
> > > > Is this a long-term solution, or a temporary one?
> > > >
> > > For upstream, I'd say we probably will stick to this solution for
> > > long time, won't say it 'forever', see comment below..
> > >
> > > > If it's a temporary conflicting one, I don't think it's worth
> > > > picking
> > > > at all.
> > > >
> > > Well, checked current Linux kernel status, there is no RPMI PM
> > > solution
> > > implemented although there is documentation in riscv-rpmi spec[1]
> >
> > I have seen pending patch for it with multiple revisions [1], the
> > dt
> > binding is even already ACKed by Conor, so I think it's not a big
> > problem here.
> >
> That's exactly what I said - it's not mature, not saying it's pending
> patch for review, but for whole solution that involve Linux driver,
> opensbi and underlying fimware (ESOS) - which not upstream-ed..
Except for the MPXY part (which seems to be already mainlined), OpenSBI
is only a RPMI consumer.
ESOS is vendor-specific thing and its version from the vendor already
contains the RPMI implementation.
The Linux driver also appears earlier and received more review than
this driver.
For not reinventing the wheel, it would be better to work on the RPMI
PM driver.
Thanks,
Icenowy
>
> To make it clear, I'm not going to pursue the RPMI solution, but if
> someone insist and willing to spend the effort, please feel free to
> push
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-19 13:56 ` Icenowy Zheng
2026-09-19 23:13 ` Yixun Lan
@ 2026-09-20 8:00 ` Troy Mitchell
1 sibling, 0 replies; 21+ messages in thread
From: Troy Mitchell @ 2026-09-20 8:00 UTC (permalink / raw)
To: Icenowy Zheng, Yixun Lan
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm,
Troy Mitchell
[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]
On Sat, Sep 19, 2026 at 09:56:12PM +0800, Icenowy Zheng wrote:
> I have seen pending patch for it with multiple revisions [1], the dt
> binding is even already ACKed by Conor, so I think it's not a big
> problem here.
>
> [1] 20260903092347.620060-1-joshua.yeong@starfivetech.com
ESOS is based on RT-Thread, and we currently have no plans to upstream
our ESOS changes to RT-Thread. Our team's scope here is upstream
open-source support; upstreaming ESOS itself is outside that scope.
The pending Linux RPMI patches are useful progress, but making the
complete solution work on K3 still requires substantial integration,
validation and maintenance work across Linux, OpenSBI and ESOS. We
cannot assume that someone in the community will take on that work.
I am not opposed to RPMI, nor does this require ESOS to be merged into
RT-Thread first. My concern is who will bring up and maintain the
complete solution. If our team is not pursuing that work and nobody
else takes it on, should power-domain support remain blocked
indefinitely?
Could we move forward with the direct-control driver, with its
mutual exclusion with ESOS power-domain management clearly documented,
and revisit RPMI when someone is ready to support that path?
- Troy
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 248 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-18 2:40 ` [PATCH 2/3] pmdomain: spacemit: Add power domain driver Yixun Lan
2026-09-18 9:00 ` Icenowy Zheng
@ 2026-09-20 12:56 ` Icenowy Zheng
2026-09-21 3:35 ` Yixun Lan
2026-10-02 18:54 ` Iker Pedrosa
2 siblings, 1 reply; 21+ messages in thread
From: Icenowy Zheng @ 2026-09-20 12:56 UTC (permalink / raw)
To: Yixun Lan, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Ulf Hansson, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti
Cc: devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> SpacemiT's PMU (Power Management Unit) consist of several power
> domains
> which can be managed independently, depending on different
> application
> scenario, each domain can be powered on/off for saving power.
>
> The driver is implemented based on Linux Generic PM Domain
> framework.
> From a hardware perspective, either of two distinct power-on
> sequences
> are supported, in software mode (SW mode), the driver is responsible
> for
> controlling the states of bits such as sleep1, sleep2, isolation and
> pwr_state, while in hardware mode (HW mode), the PMU hardware will
> complete
> the sequence automatically without requiring software intervention.
This driver is tested to be able to power up the GPU on SpacemiT K3,
however the following warning appears in the kernel log:
```
genpd_provider audio: Fail to power-off domain: 2
```
(BTW I built it as a module to prevent whole kernel being rebuilt)
Thanks,
Icenowy
>
> Signed-off-by: Yixun Lan <dlan@kernel.org>
> ---
> drivers/pmdomain/Kconfig | 1 +
> drivers/pmdomain/Makefile | 1 +
> drivers/pmdomain/spacemit/Kconfig | 15 ++
> drivers/pmdomain/spacemit/Makefile | 1 +
> drivers/pmdomain/spacemit/pm_domains.c | 448
> +++++++++++++++++++++++++++++++++
> 5 files changed, 466 insertions(+)
>
> diff --git a/drivers/pmdomain/Kconfig b/drivers/pmdomain/Kconfig
> index 23076ae90e66..0610acd030f6 100644
> --- a/drivers/pmdomain/Kconfig
> +++ b/drivers/pmdomain/Kconfig
> @@ -13,6 +13,7 @@ source "drivers/pmdomain/qcom/Kconfig"
> source "drivers/pmdomain/renesas/Kconfig"
> source "drivers/pmdomain/rockchip/Kconfig"
> source "drivers/pmdomain/samsung/Kconfig"
> +source "drivers/pmdomain/spacemit/Kconfig"
> source "drivers/pmdomain/st/Kconfig"
> source "drivers/pmdomain/starfive/Kconfig"
> source "drivers/pmdomain/sunxi/Kconfig"
> diff --git a/drivers/pmdomain/Makefile b/drivers/pmdomain/Makefile
> index ebc802f13eb9..ba25b444e263 100644
> --- a/drivers/pmdomain/Makefile
> +++ b/drivers/pmdomain/Makefile
> @@ -11,6 +11,7 @@ obj-y += qcom/
> obj-y += renesas/
> obj-y += rockchip/
> obj-y += samsung/
> +obj-y += spacemit/
> obj-y += st/
> obj-y += starfive/
> obj-y += sunxi/
> diff --git a/drivers/pmdomain/spacemit/Kconfig
> b/drivers/pmdomain/spacemit/Kconfig
> new file mode 100644
> index 000000000000..4b927beedc7c
> --- /dev/null
> +++ b/drivers/pmdomain/spacemit/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +menu "SpacemiT PM Domains"
> +
> +config SPACEMIT_PM_DOMAINS
> + bool "PM Domain driver for SpacemiT SoC"
> + depends on PM
> + depends on OF
> + depends on MFD_SYSCON
> + select PM_GENERIC_DOMAINS
> + help
> + PM Domain driver for SpacemiT SoC. Say Y if you want to
> support
> + SpacemiT SoCs.
> +
> +endmenu
> diff --git a/drivers/pmdomain/spacemit/Makefile
> b/drivers/pmdomain/spacemit/Makefile
> new file mode 100644
> index 000000000000..5e8a740ba494
> --- /dev/null
> +++ b/drivers/pmdomain/spacemit/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_SPACEMIT_PM_DOMAINS) += pm_domains.o
> 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
> @@ -0,0 +1,448 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Spacemit Generic power domain support.
> + *
> + * Copyright (c) 2026 SpacemiT Technology Co. Ltd
> + */
> +
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <dt-bindings/power/spacemit,k1-power.h>
> +#include <dt-bindings/power/spacemit,k3-power.h>
> +
> +#define APMU_POWER_STATUS_REG 0xf0
> +#define APMU_POWER_TIMEOUT_US 10000
> +
> +struct spacemit_pm_domain_param {
> + int reg_pwr_ctrl;
> + int bit_hw_mode;
> + int bit_sleep2;
> + int bit_sleep1;
> + int bit_isolation;
> + int bit_auto_pwr_on;
> + int bit_hw_pwr_stat;
> + int bit_pwr_stat;
> + int use_hw;
> + const char *name;
> +};
> +
> +struct spacemit_pm_domain {
> + struct generic_pm_domain genpd;
> + int pm_index;
> + const struct spacemit_pm_domain_param *param;
> +};
> +
> +struct spacemit_pmu {
> + struct device *dev;
> + struct genpd_onecell_data genpd_data;
> + struct regmap *regmap;
> + struct spacemit_pm_domain **domains;
> + int num_domains;
> +};
> +
> +struct spacemit_pm_of_data {
> + int num_domains;
> + const struct spacemit_pm_domain_param *param;
> +};
> +
> +static struct spacemit_pmu *gpmu;
> +
> +static int spacemit_pd_power_off(struct generic_pm_domain *domain)
> +{
> + struct spacemit_pm_domain *spd = container_of(domain, struct
> spacemit_pm_domain, genpd);
> + const struct spacemit_pm_domain_param *p = spd->param;
> + unsigned int val;
> + int ret;
> +
> + if (!spd->param->use_hw) {
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> BIT(p->bit_isolation));
> + fsleep(15);
> +
> + val = BIT(p->bit_sleep1) | BIT(p->bit_sleep2);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(15);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + !(val & BIT(p-
> >bit_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + } else {
> + val = BIT(p->bit_auto_pwr_on) | BIT(p->bit_hw_mode);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(15);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + !(val & BIT(p-
> >bit_hw_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + }
> +
> + if (ret) {
> + dev_err(&domain->dev, "Fail to power-off domain:
> %d\n",
> + spd->pm_index);
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
> +static int spacemit_pd_power_on(struct generic_pm_domain *domain)
> +{
> + struct spacemit_pm_domain *spd = container_of(domain, struct
> spacemit_pm_domain, genpd);
> + const struct spacemit_pm_domain_param *p = spd->param;
> + unsigned int val;
> + int ret = 0;
> +
> + regmap_read(gpmu->regmap, APMU_POWER_STATUS_REG, &val);
> +
> + if (!p->use_hw && (val & BIT(p->bit_pwr_stat))) {
> + val = BIT(p->bit_isolation);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(15);
> +
> + val = BIT(p->bit_sleep1) | BIT(p->bit_sleep2);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(15);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + !(val & BIT(p-
> >bit_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + }
> +
> + if (p->use_hw && (val & BIT(p->bit_hw_pwr_stat))) {
> + val = BIT(p->bit_auto_pwr_on) | BIT(p->bit_hw_mode);
> + regmap_clear_bits(gpmu->regmap, p->reg_pwr_ctrl,
> val);
> + fsleep(30);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + !(val & BIT(p-
> >bit_hw_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + }
> +
> + if (ret < 0) {
> + dev_err(&domain->dev, "power-off domain: %d,
> error\n", spd->pm_index);
> + return -EBUSY;
> + }
> +
> + if (!p->use_hw) {
> + regmap_set_bits(gpmu->regmap, p->reg_pwr_ctrl,
> BIT(p->bit_sleep1));
> + fsleep(20);
> +
> + val = BIT(p->bit_sleep2) | BIT(p->bit_sleep1);
> + regmap_set_bits(gpmu->regmap, p->reg_pwr_ctrl, val);
> + fsleep(20);
> +
> + regmap_set_bits(gpmu->regmap, p->reg_pwr_ctrl,
> BIT(p->bit_isolation));
> + fsleep(15);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + (val & BIT(p-
> >bit_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + } else {
> + val = BIT(p->bit_auto_pwr_on) | BIT(p->bit_hw_mode);
> + regmap_set_bits(gpmu->regmap, p->reg_pwr_ctrl, val);
> + fsleep(300);
> +
> + ret = regmap_read_poll_timeout(gpmu->regmap,
> +
> APMU_POWER_STATUS_REG,
> + val,
> + (val & BIT(p-
> >bit_hw_pwr_stat)),
> + 5,
> +
> APMU_POWER_TIMEOUT_US);
> + }
> +
> + if (ret < 0) {
> + dev_err(&domain->dev, "power-on domain: %d,
> error\n", spd->pm_index);
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
> +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, ®);
> +
> + bit = p->use_hw ? BIT(pd->param->bit_hw_pwr_stat) :
> + BIT(pd->param->bit_pwr_stat);
> +
> + return !!(reg & bit);
> +}
> +
> +static int spacemit_pm_add_one_domain(struct spacemit_pmu *pmu, int
> id,
> + const struct
> spacemit_pm_domain_param *param)
> +{
> + struct spacemit_pm_domain *pd;
> +
> + pd = devm_kzalloc(pmu->dev, sizeof(*pd), GFP_KERNEL);
> + if (!pd)
> + return -ENOMEM;
> +
> + pd->pm_index = id;
> + pd->param = param;
> + pd->genpd.name = param->name;
> + pd->genpd.power_off = spacemit_pd_power_off;
> + pd->genpd.power_on = spacemit_pd_power_on;
> +
> + pm_genpd_init(&pd->genpd, NULL, !spacemit_pm_get_state(pmu,
> pd));
> +
> + pmu->domains[id] = pd;
> +
> + return 0;
> +}
> +
> +static void spacemit_pm_domain_cleanup(struct spacemit_pmu *pmu)
> +{
> + int i;
> +
> + for (i = 0; i < pmu->num_domains; i++) {
> + if (pmu->domains[i])
> + pm_genpd_remove(&pmu->domains[i]->genpd);
> + }
> +}
> +
> +static int spacemit_pm_domain_probe(struct platform_device *pdev)
> +{
> + const struct spacemit_pm_of_data *data;
> + struct device *dev = &pdev->dev;
> + struct spacemit_pmu *pmu;
> + int err, i;
> +
> + data = device_get_match_data(dev);
> +
> + pmu = devm_kzalloc(dev, sizeof(*pmu), GFP_KERNEL);
> + if (!pmu)
> + return -ENOMEM;
> +
> + pmu->dev = dev;
> + pmu->num_domains = data->num_domains;
> +
> + pmu->regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
> "spacemit,apmu");
> + if (IS_ERR(pmu->regmap))
> + return dev_err_probe(dev, PTR_ERR(pmu->regmap),
> "failed to get apmu regmap\n");
> +
> + pmu->domains = devm_kcalloc(dev, data->num_domains,
> + sizeof(*pmu->domains),
> GFP_KERNEL);
> + if (!pmu->domains)
> + return -ENOMEM;
> +
> + for (i = 0; i < data->num_domains; i++) {
> + err = spacemit_pm_add_one_domain(pmu, i, &data-
> >param[i]);
> + if (err) {
> + dev_err(dev, "failed to add domain %d:
> %d\n", i, err);
> + goto err_out;
> + }
> + }
> +
> + pmu->genpd_data.domains = (struct generic_pm_domain
> **)pmu->domains;
> + pmu->genpd_data.num_domains = data->num_domains;
> +
> + err = of_genpd_add_provider_onecell(dev->of_node, &pmu-
> >genpd_data);
> + if (err) {
> + dev_err(dev, "failed to add provider: %d\n", err);
> + goto err_out;
> + }
> +
> + gpmu = pmu;
> +
> + return 0;
> +
> +err_out:
> + spacemit_pm_domain_cleanup(pmu);
> + return err;
> +}
> +
> +static const struct spacemit_pm_domain_param k1_domain_params[] = {
> + [K1_PMDOMAIN_VPU] = {
> + .reg_pwr_ctrl = 0xa8,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_pwr_stat = 1,
> + .bit_hw_pwr_stat = 9,
> + .name = "vpu",
> + },
> + [K1_PMDOMAIN_GPU] = {
> + .reg_pwr_ctrl = 0xd0,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_pwr_stat = 0,
> + .name = "gpu",
> + },
> + [K1_PMDOMAIN_LCD] = {
> + .reg_pwr_ctrl = 0x380,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 4,
> + .bit_hw_pwr_stat = 12,
> + .use_hw = 1,
> + .name = "lcd",
> + },
> + [K1_PMDOMAIN_ISP] = {
> + .reg_pwr_ctrl = 0x37c,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 2,
> + .bit_hw_pwr_stat = 10,
> + .name = "isp",
> + },
> + [K1_PMDOMAIN_AUDIO] = {
> + .reg_pwr_ctrl = 0x378,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 3,
> + .bit_hw_pwr_stat = 11,
> + .use_hw = 1,
> + .name = "audio",
> + },
> + [K1_PMDOMAIN_GNSS] = {
> + .reg_pwr_ctrl = 0x13c,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 6,
> + .bit_hw_pwr_stat = 14,
> + .name = "gnss",
> + },
> + [K1_PMDOMAIN_HDMI] = {
> + .reg_pwr_ctrl = 0x3f4,
> + .bit_hw_mode = 4,
> + .bit_sleep2 = 3,
> + .bit_sleep1 = 2,
> + .bit_isolation = 1,
> + .bit_auto_pwr_on = 0,
> + .bit_pwr_stat = 7,
> + .bit_hw_pwr_stat = 15,
> + .use_hw = 1,
> + .name = "hdmi",
> + },
> +};
> +
> +static const struct spacemit_pm_domain_param k3_domain_params[] = {
> + [K3_PMDOMAIN_VPU] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 2,
> + .bit_hw_pwr_stat = 9,
> + .use_hw = 1,
> + .reg_pwr_ctrl = 0xa8,
> + .name = "vpu",
> + },
> + [K3_PMDOMAIN_GPU] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 0,
> + .bit_hw_pwr_stat = 8,
> + .use_hw = 1,
> + .reg_pwr_ctrl = 0xd0,
> + .name = "gpu",
> + },
> + [K3_PMDOMAIN_AUDIO] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 3,
> + .reg_pwr_ctrl = 0x378,
> + .name = "audio",
> + },
> + [K3_PMDOMAIN_LCD0] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 4,
> + .bit_hw_pwr_stat = 12,
> + .use_hw = 1,
> + .reg_pwr_ctrl = 0x380,
> + .name = "lcd0",
> + },
> + [K3_PMDOMAIN_LCD1] = {
> + .bit_auto_pwr_on = 0,
> + .bit_isolation = 1,
> + .bit_sleep1 = 2,
> + .bit_sleep2 = 3,
> + .bit_hw_mode = 4,
> + .bit_pwr_stat = 5,
> + .bit_hw_pwr_stat = 15,
> + .use_hw = 1,
> + .reg_pwr_ctrl = 0x3f4,
> + .name = "lcd1",
> + },
> +};
> +
> +static const struct spacemit_pm_of_data k1_of_data = {
> + .num_domains = ARRAY_SIZE(k1_domain_params),
> + .param = k1_domain_params,
> +};
> +
> +static const struct spacemit_pm_of_data k3_of_data = {
> + .num_domains = ARRAY_SIZE(k3_domain_params),
> + .param = k3_domain_params,
> +};
> +
> +static const struct of_device_id spacemit_pm_domain_dt_match[] = {
> + {
> + .compatible = "spacemit,k1-power-controller",
> + .data = &k1_of_data,
> + },
> + {
> + .compatible = "spacemit,k3-power-controller",
> + .data = &k3_of_data,
> + },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver spacemit_pm_domain_driver = {
> + .probe = spacemit_pm_domain_probe,
> + .driver = {
> + .name = "spacemit-pm-domain",
> + .of_match_table = spacemit_pm_domain_dt_match,
> + },
> +};
> +
> +builtin_platform_driver(spacemit_pm_domain_driver);
> +MODULE_DESCRIPTION("SpacemiT Generic Power Domains driver");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-20 12:56 ` Icenowy Zheng
@ 2026-09-21 3:35 ` Yixun Lan
2026-09-21 3:39 ` Icenowy Zheng
2026-09-21 3:41 ` Icenowy Zheng
0 siblings, 2 replies; 21+ messages in thread
From: Yixun Lan @ 2026-09-21 3:35 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
Hi Icenowy,
On 20:56 Sun 20 Sep , Icenowy Zheng wrote:
> 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > SpacemiT's PMU (Power Management Unit) consist of several power
> > domains
> > which can be managed independently, depending on different
> > application
> > scenario, each domain can be powered on/off for saving power.
> >
> > The driver is implemented based on Linux Generic PM Domain
> > framework.
> > From a hardware perspective, either of two distinct power-on
> > sequences
> > are supported, in software mode (SW mode), the driver is responsible
> > for
> > controlling the states of bits such as sleep1, sleep2, isolation and
> > pwr_state, while in hardware mode (HW mode), the PMU hardware will
> > complete
> > the sequence automatically without requiring software intervention.
>
> This driver is tested to be able to power up the GPU on SpacemiT K3,
> however the following warning appears in the kernel log:
>
> ```
> genpd_provider audio: Fail to power-off domain: 2
> ```
>
This is kind of an expected issue, due to hardware design, the audio
power switch is coupled with rcpu core, which means power off it will
also shut down the rcpu, so the power switch currently is configured
as always on from software perspective, it's unable to power-off even
in Linux or rcpu once system up.
I will drop audio power domain control in next version
> (BTW I built it as a module to prevent whole kernel being rebuilt)
>
that's fine
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-21 3:35 ` Yixun Lan
@ 2026-09-21 3:39 ` Icenowy Zheng
2026-09-21 12:43 ` Yixun Lan
2026-09-21 3:41 ` Icenowy Zheng
1 sibling, 1 reply; 21+ messages in thread
From: Icenowy Zheng @ 2026-09-21 3:39 UTC (permalink / raw)
To: Yixun Lan
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
在 2026-09-21一的 03:35 +0000，Yixun Lan写道：
> Hi Icenowy,
>
> On 20:56 Sun 20 Sep , Icenowy Zheng wrote:
> > 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > > SpacemiT's PMU (Power Management Unit) consist of several power
> > > domains
> > > which can be managed independently, depending on different
> > > application
> > > scenario, each domain can be powered on/off for saving power.
> > >
> > > The driver is implemented based on Linux Generic PM Domain
> > > framework.
> > > From a hardware perspective, either of two distinct power-on
> > > sequences
> > > are supported, in software mode (SW mode), the driver is
> > > responsible
> > > for
> > > controlling the states of bits such as sleep1, sleep2, isolation
> > > and
> > > pwr_state, while in hardware mode (HW mode), the PMU hardware
> > > will
> > > complete
> > > the sequence automatically without requiring software
> > > intervention.
> >
> > This driver is tested to be able to power up the GPU on SpacemiT
> > K3,
> > however the following warning appears in the kernel log:
> >
> > ```
> > genpd_provider audio: Fail to power-off domain: 2
> > ```
> >
> This is kind of an expected issue, due to hardware design, the audio
> power switch is coupled with rcpu core, which means power off it will
> also shut down the rcpu, so the power switch currently is configured
> as always on from software perspective, it's unable to power-off even
> in Linux or rcpu once system up.
Should the GENPD_FLAG_ALWAYS_ON flag be applied onto this power domain
if it's not meant to be shut down?
Thanks,
Icenowy
>
> I will drop audio power domain control in next version
>
> > (BTW I built it as a module to prevent whole kernel being rebuilt)
> >
> that's fine
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-21 3:39 ` Icenowy Zheng
@ 2026-09-21 12:43 ` Yixun Lan
0 siblings, 0 replies; 21+ messages in thread
From: Yixun Lan @ 2026-09-21 12:43 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
Hi Icenowy,
On 11:39 Mon 21 Sep , Icenowy Zheng wrote:
> 在 2026-09-21一的 03:35 +0000，Yixun Lan写道：
..
>
> Should the GENPD_FLAG_ALWAYS_ON flag be applied onto this power domain
> if it's not meant to be shut down?
>
Yes, I think this is better approach, thanks
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-21 3:35 ` Yixun Lan
2026-09-21 3:39 ` Icenowy Zheng
@ 2026-09-21 3:41 ` Icenowy Zheng
2026-09-21 12:44 ` Yixun Lan
1 sibling, 1 reply; 21+ messages in thread
From: Icenowy Zheng @ 2026-09-21 3:41 UTC (permalink / raw)
To: Yixun Lan
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
在 2026-09-21一的 03:35 +0000，Yixun Lan写道：
> Hi Icenowy,
>
> On 20:56 Sun 20 Sep , Icenowy Zheng wrote:
> > 在 2026-09-18五的 02:40 +0000，Yixun Lan写道：
> > > SpacemiT's PMU (Power Management Unit) consist of several power
> > > domains
> > > which can be managed independently, depending on different
> > > application
> > > scenario, each domain can be powered on/off for saving power.
> > >
> > > The driver is implemented based on Linux Generic PM Domain
> > > framework.
> > > From a hardware perspective, either of two distinct power-on
> > > sequences
> > > are supported, in software mode (SW mode), the driver is
> > > responsible
> > > for
> > > controlling the states of bits such as sleep1, sleep2, isolation
> > > and
> > > pwr_state, while in hardware mode (HW mode), the PMU hardware
> > > will
> > > complete
> > > the sequence automatically without requiring software
> > > intervention.
> >
> > This driver is tested to be able to power up the GPU on SpacemiT
> > K3,
> > however the following warning appears in the kernel log:
> >
> > ```
> > genpd_provider audio: Fail to power-off domain: 2
> > ```
> >
> This is kind of an expected issue, due to hardware design, the audio
> power switch is coupled with rcpu core, which means power off it will
> also shut down the rcpu, so the power switch currently is configured
> as always on from software perspective, it's unable to power-off even
> in Linux or rcpu once system up.
>
> I will drop audio power domain control in next version
>
> > (BTW I built it as a module to prevent whole kernel being rebuilt)
> >
> that's fine
In this case, do you consider to make the Kconfig option in your patch
tristate instead of bool?
Thanks,
Icenowy
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-21 3:41 ` Icenowy Zheng
@ 2026-09-21 12:44 ` Yixun Lan
0 siblings, 0 replies; 21+ messages in thread
From: Yixun Lan @ 2026-09-21 12:44 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
Hi Icenowy,
On 11:41 Mon 21 Sep , Icenowy Zheng wrote:
> 在 2026-09-21一的 03:35 +0000，Yixun Lan写道：
..
> > > (BTW I built it as a module to prevent whole kernel being rebuilt)
> > >
> > that's fine
>
> In this case, do you consider to make the Kconfig option in your patch
> tristate instead of bool?
>
Sure, will do
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-09-18 2:40 ` [PATCH 2/3] pmdomain: spacemit: Add power domain driver Yixun Lan
2026-09-18 9:00 ` Icenowy Zheng
2026-09-20 12:56 ` Icenowy Zheng
@ 2026-10-02 18:54 ` Iker Pedrosa
2026-10-04 13:47 ` Yixun Lan
2 siblings, 1 reply; 21+ messages in thread
From: Iker Pedrosa @ 2026-10-02 18:54 UTC (permalink / raw)
To: Yixun Lan
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
El vie, 18 sept 2026 a las 7:49, Yixun Lan (<dlan@kernel.org>) 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, ®);
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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/3] pmdomain: spacemit: Add power domain driver
2026-10-02 18:54 ` Iker Pedrosa
@ 2026-10-04 13:47 ` Yixun Lan
0 siblings, 0 replies; 21+ messages in thread
From: Yixun Lan @ 2026-10-04 13:47 UTC (permalink / raw)
To: Iker Pedrosa
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
devicetree, linux-riscv, spacemit, linux-kernel, linux-pm
Hi Iker Pedrosa,
On 20:54 Fri 02 Oct , Iker Pedrosa wrote:
> El vie, 18 sept 2026 a las 7:49, Yixun Lan (<dlan@kernel.org>) 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()
>
It's not necessary, or it's not really useful to check return value here,
regmap_read() shouldn't fail here, it generally access the io memory
> > [...]
> > + 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()?
>
will drop this logic, I just followed vendor driver which try to force power
off donmain before doing the power on operation if it's already in 'on' state..
while found it's not really necessary to do this during actual testing
> > [...]
> > +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, ®);
>
> Please check the return value here as well
>
I don't think the check is useful, in rare case it should fail
> > + 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
Yes, will do
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 21+ messages in thread