Re: [PATCH v7 05/10] pmdomain: samsung: convert to using regmap

[Peter Griffin]

On Fri, 6 Mar 2026 at 10:29, André Draszik <andre.draszik@xxxxxxxxxx> wrote:
>
> On platforms such as Google gs101, direct mmio register access to the
> PMU registers doesn't necessarily work and access must happen via a
> regmap created by the PMU driver instead.
>
> In preparation for supporting such SoCs convert the existing mmio
> accesses to using a regmap wrapper.
>
> With this change in place, a follow-up patch can update the driver to
> optionally acquire the PMU-created regmap without having to change the
> rest of the code.
>
> Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> Signed-off-by: André Draszik <andre.draszik@xxxxxxxxxx>
> ---
> There is one checkpatch warning, relating to the non-const
> regmap_config. It can not easily be made const at this stage, but a
> follow-up patch allows us to make it const and the warning will go away
> anyway.
>
> v4:
> - rework the loop in exynos_pd_power() slightly, to not return 0 early
>   to allow more code to be run after pd on/off register write without
>   changing the loop again, required for gs101.
> - add error message in case first regmap write in exynos_pd_power() fails
> ---

Reviewed-by: Peter Griffin <peter.griffin@xxxxxxxxxx>

>  drivers/pmdomain/samsung/exynos-pm-domains.c | 83 +++++++++++++++++++++-------
>  1 file changed, 62 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
> index 5c3aa8983087..3bcba7d38ac1 100644
> --- a/drivers/pmdomain/samsung/exynos-pm-domains.c
> +++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
> @@ -9,15 +9,14 @@
>  // conjunction with runtime-pm. Support for both device-tree and non-device-tree
>  // based power domain support is included.
>
> -#include <linux/io.h>
>  #include <linux/err.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/pm_domain.h>
>  #include <linux/delay.h>
>  #include <linux/of.h>
> -#include <linux/of_address.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
>
>  struct exynos_pm_domain_config {
>         /* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
> @@ -28,7 +27,7 @@ struct exynos_pm_domain_config {
>   * Exynos specific wrapper around the generic power domain
>   */
>  struct exynos_pm_domain {
> -       void __iomem *base;
> +       struct regmap *regmap;
>         struct generic_pm_domain pd;
>         u32 local_pwr_cfg;
>  };
> @@ -36,31 +35,42 @@ struct exynos_pm_domain {
>  static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
>  {
>         struct exynos_pm_domain *pd;
> -       void __iomem *base;
>         u32 timeout, pwr;
> -       char *op;
> +       int err;
>
>         pd = container_of(domain, struct exynos_pm_domain, pd);
> -       base = pd->base;
>
>         pwr = power_on ? pd->local_pwr_cfg : 0;
> -       writel_relaxed(pwr, base);
> +       err = regmap_write(pd->regmap, 0, pwr);
> +       if (err) {
> +               pr_err("Regmap write for power domain %s %sable failed: %d\n",
> +                      domain->name, power_on ? "en" : "dis", err);
> +               return err;
> +       }
>
>         /* Wait max 1ms */
>         timeout = 10;
> -
> -       while ((readl_relaxed(base + 0x4) & pd->local_pwr_cfg) != pwr) {
> -               if (!timeout) {
> -                       op = (power_on) ? "enable" : "disable";
> -                       pr_err("Power domain %s %s failed\n", domain->name, op);
> -                       return -ETIMEDOUT;
> +       while (timeout-- > 0) {
> +               unsigned int val;
> +
> +               err = regmap_read(pd->regmap, 0x4, &val);
> +               if (err || ((val & pd->local_pwr_cfg) != pwr)) {
> +                       cpu_relax();
> +                       usleep_range(80, 100);
> +                       continue;
>                 }
> -               timeout--;
> -               cpu_relax();
> -               usleep_range(80, 100);
> +
> +               break;
>         }
>
> -       return 0;
> +       if (!timeout && !err)
> +               /* Only return timeout if no other error also occurred. */
> +               err = -ETIMEDOUT;
> +       if (err)
> +               pr_err("Power domain %s %sable failed: %d\n", domain->name,
> +                      power_on ? "en" : "dis", err);
> +
> +       return err;
>  }
>
>  static int exynos_pd_power_on(struct generic_pm_domain *domain)
> @@ -109,8 +119,18 @@ static int exynos_pd_probe(struct platform_device *pdev)
>         struct device_node *np = dev->of_node;
>         struct of_phandle_args child, parent;
>         struct exynos_pm_domain *pd;
> +       struct resource *res;
> +       void __iomem *base;
> +       unsigned int val;
>         int on, ret;
>
> +       struct regmap_config reg_config = {
> +               .reg_bits = 32,
> +               .val_bits = 32,
> +               .reg_stride = 4,
> +               .use_relaxed_mmio = true,
> +       };
> +
>         pm_domain_cfg = of_device_get_match_data(dev);
>         pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
>         if (!pd)
> @@ -120,9 +140,26 @@ static int exynos_pd_probe(struct platform_device *pdev)
>         if (!pd->pd.name)
>                 return -ENOMEM;
>
> -       pd->base = of_iomap(np, 0);
> -       if (!pd->base)
> -               return -ENODEV;
> +       /*
> +        * The resource typically points into the address space of the PMU.
> +        * Therefore, avoid using devm_platform_get_and_ioremap_resource() and
> +        * instead use platform_get_resource() and devm_ioremap() to avoid
> +        * conflicts due to address space overlap.
> +        */
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return dev_err_probe(dev, -ENXIO, "missing IO resources");
> +
> +       base = devm_ioremap(dev, res->start, resource_size(res));
> +       if (!base)
> +               return dev_err_probe(dev, -ENOMEM,
> +                                    "failed to ioremap PMU registers");
> +
> +       reg_config.max_register = resource_size(res) - reg_config.reg_stride;
> +       pd->regmap = devm_regmap_init_mmio(dev, base, &reg_config);
> +       if (IS_ERR(pd->regmap))
> +               return dev_err_probe(dev, PTR_ERR(base),
> +                                    "failed to init regmap");
>
>         pd->pd.power_off = exynos_pd_power_off;
>         pd->pd.power_on = exynos_pd_power_on;
> @@ -137,7 +174,11 @@ static int exynos_pd_probe(struct platform_device *pdev)
>             of_device_is_compatible(np, "samsung,exynos4210-pd"))
>                 exynos_pd_power_off(&pd->pd);
>
> -       on = readl_relaxed(pd->base + 0x4) & pd->local_pwr_cfg;
> +       ret = regmap_read(pd->regmap, 0x4, &val);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to read status");
> +
> +       on = val & pd->local_pwr_cfg;
>
>         pm_genpd_init(&pd->pd, NULL, !on);
>         ret = of_genpd_add_provider_simple(np, &pd->pd);
>
> --
> 2.53.0.473.g4a7958ca14-goog
>