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

[Marek Szyprowski]

On 19.03.2026 11:13, Ulf Hansson wrote:
> On Wed, 18 Mar 2026 at 16:28, 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>
> [...]
>
>> @@ -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;
>>          }
>>
> [...]
>
> As a follow-up patch on top, please consider converting the open-coded
> polling loop above into a readx_poll_timeout_atomic().

This has been tried and it doesn't work in all cases required for power 
domain driver:

https://lore.kernel.org/all/5c19e4ef-c4fd-4bf5-88b3-46c86751b14e@xxxxxxxxxxx/

Probably a comment about that could be added directly to this code to 
avoid such conversion and breakage in the future.

> That said, the series looks ready to me, but I am awaiting an ack from
> a DT maintainer on patch4 before applying.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland