[PATCH RFC v2 09/16] arm: domain: Add platform callbacks for domain power on/off

Geert Uytterhoeven geert at linux-m68k.org
Tue Jun 30 08:10:36 PDT 2015


Hi Lina,

On Mon, Jun 29, 2015 at 6:32 PM, Lina Iyer <lina.iyer at linaro.org> wrote:
> On Mon, Jun 29 2015 at 07:36 -0600, Geert Uytterhoeven wrote:
>>> --- /dev/null
>>> +++ b/arch/arm/include/asm/pm_domain.h
>>> @@ -0,0 +1,27 @@
>>> +/*
>>> + *  arch/arm/include/asm/pm_domain.h
>>> + *
>>> + *  Copyright (C) 2015 Linaro Ltd.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +#ifndef __ASM_ARM_PM_DOMAIN_H
>>> +#define __ASM_ARM_PM_DOMAIN_H
>>> +
>>> +#include <linux/pm_domain.h>
>>> +
>>> +#if IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)
>>> +extern int register_platform_domain_handlers(struct of_phandle_args
>>> *args,
>>> +               int (*pd_down)(struct generic_pm_domain *),
>>> +               int (*pd_up)(struct generic_pm_domain *));
>>
>>
>> This looks a bit convoluted to me...
>>
>>> --- a/arch/arm/kernel/domains.c
>>> +++ b/arch/arm/kernel/domains.c
>>> @@ -9,10 +9,19 @@
>>>  #include <linux/module.h>
>>>  #include <linux/platform_device.h>
>>>
>>> +#include <asm/pm_domain.h>
>>> +
>>>  #define NAME_MAX 16
>>>
>>> +struct platform_cb {
>>> +       int (*power_off)(struct generic_pm_domain *);
>>> +       int (*power_on)(struct generic_pm_domain *);
>>> +};
>>> +
>>>  struct arm_pm_domain {
>>>         struct generic_pm_domain genpd;
>>> +       struct platform_cb plat_handler;
>>> +       struct spinlock_t lock;
>>>  };
>>>
>>>  static inline
>>> @@ -23,16 +32,85 @@ struct arm_pm_domain *to_arm_pd(struct
>>> generic_pm_domain *d)
>>>
>>>  static int arm_pd_power_down(struct generic_pm_domain *genpd)
>>>  {
>>> +       struct arm_pm_domain *arm_pd = to_arm_pd(genpd);
>>> +
>>> +       if (arm_pd->plat_handler.power_off)
>>> +               return arm_pd->plat_handler.power_off(genpd);
>>> +
>>>         /* pr_info("KJH: %s: %s\n", __func__, genpd->name); */
>>>         return 0;
>>>  }
>>>
>>>  static int arm_pd_power_up(struct generic_pm_domain *genpd)
>>>  {
>>> +       struct arm_pm_domain *arm_pd = to_arm_pd(genpd);
>>> +
>>> +       if (arm_pd->plat_handler.power_on)
>>> +               return arm_pd->plat_handler.power_on(genpd);
>>> +
>>>         /* pr_info("KJH: %s: %s\n", __func__, genpd->name); */
>>>         return 0;
>>>  }
>>
>>
>>> @@ -152,6 +230,7 @@ static int arm_domain_init(void)
>>>                 pd->genpd.power_off = arm_pd_power_down;
>>>                 pd->genpd.power_on = arm_pd_power_up;
>>
>>
>> Shouldn't these .power_off() and .power_on() be set up from platform code
>> instead, in the platform-specific code that creates the PM domain?
>>
>> The PM Domain containing the CPU may contain other devices, in which
>> case it's already set up from platform-specific code, which would conflict
>> with arm_domain_init()?
>>
> In my first RFC, the platform code was creating a domain. In this RFC,
> we are flaunting an idea that a generic code could setup the domains
> without any intervention from platform code. It will do everything
> common on most ARM platforms. Architectures that do not have anything
> specific in their domain, would benefit from such an initialization.
>
> If we were to export this genpd and then platform code could attach more
> devices to the genpd, or make it a sub of another genpd, would that
> work?
>
>> Cfr. arch/arm/mach-shmobile/pm-rmobile.c, which handles all PM domains
>> (both for devices and CPUs) on R-Mobile, and
>> arch/arm/boot/dts/{r8a73a4,r8a7740,sh73a0}.dtsi.
>>
>> R8a73a4 is the most advanced of these three: it has 2 big.LITTLE clusters,
>> with separate PM Domains for L2/SCU and sub-domains for the CPUs.
>
>
> If you could get the CPU genpd from the ARM common code, you could embed
> that in the L2 domain.
>
> On QCOM's big.LITTLE SoC, there would a PM domain for each of the CPUs
> and then there would be a coherency PM domain that would contain the big
> and LITTLE cluster domains. In that case, the platform code would create
> and initialize the coherency domain and make the CPU domains a sub of
> the coherency domain. Would something like that work?
>
>
>> Unfortunately we don't have SMP support for it, so currently dtsi
>> describes
>> the first cpu core only. The full structure should look like this
>>
>>        cpus {
>>                cpu0: cpu at 0 {
>>                        compatible = "arm,cortex-a15";
>>                        power-domains = <&pd_a2sl>;
>>                        next-level-cache = <&L2_CA15>;
>>                };
>>
>>                cpu1: cpu at 1 {
>>                        compatible = "arm,cortex-a15";
>>                        power-domains = <&pd_a2sl>;
>>                        next-level-cache = <&L2_CA15>;
>>                };
>>
>>                cpu2: cpu at 2 {
>>                        compatible = "arm,cortex-a15";
>>                        power-domains = <&pd_a2sl>;
>>                        next-level-cache = <&L2_CA15>;
>>                };
>>
>>                cpu3: cpu at 3 {
>>                        compatible = "arm,cortex-a15";
>>                        power-domains = <&pd_a2sl>;
>>                        next-level-cache = <&L2_CA15>;
>>                };
>>
>>                cpu4: cpu at 4 {
>>                        compatible = "arm,cortex-a7";
>>                        power-domains = <&pd_a2kl>;
>>                        next-level-cache = <&L2_CA7>;
>>                };
>>
>>                cpu5: cpu at 5 {
>>                        compatible = "arm,cortex-a7";
>>                        power-domains = <&pd_a2kl>;
>>                        next-level-cache = <&L2_CA7>;
>>                };
>>
>>                cpu6: cpu at 6 {
>>                        compatible = "arm,cortex-a7";
>>                        power-domains = <&pd_a2kl>;
>>                        next-level-cache = <&L2_CA7>;
>>                };
>>
>>                cpu7: cpu at 7 {
>>                        compatible = "arm,cortex-a7";
>>                        power-domains = <&pd_a2kl>;
>>                        next-level-cache = <&L2_CA7>;
>>                };
>>        };
>>
>>        L2_CA15: cache-controller at 0 {
>>                compatible = "cache";
>>                power-domains = <&pd_a3sm>;
>>        };
>>
>>        L2_CA7: cache-controller at 1 {
>>                compatible = "cache";
>>                power-domains = <&pd_a3km>;
>>        };
>>
>> And the PM Domain part (which is complete in upstream):
>>
>>        pd_c4: c4 at 0 {
>>                #power-domain-cells = <0>;
>>
>>                pd_a3sm: a3sm at 20 {
>>                        reg = <20>;
>>                        #power-domain-cells = <0>;
>>
>>                        pd_a2sl: a2sl at 21 {
>>                                reg = <21>;
>>                                #power-domain-cells = <0>;
>>                        };
>>                };
>>
>>                pd_a3km: a3km at 22 {
>>                        reg = <22>;
>>                        #size-cells = <0>;
>>                        #power-domain-cells = <0>;
>>
>>                        pd_a2kl: a2kl at 23 {
>>                                reg = <23>;
>>                                #power-domain-cells = <0>;
>>                        };
>>                };
>>        };
>>
> Thanks for the example. Would it work, if the platform code initalizes
> the pd_a3sm, pd_a3km and pd_c4 and set up the hierarchy and to add the
> CPU domains, you could query the ARM and then add the domains to the
> pd_a3sm and pd_a3km?

Yes, it could work. But then I have to add _more_ code to ignore the a2sl
and a2kl code in pm-rmobile.c, and call register_platform_domain_handlers(),
as in the end it's pm-rmobile that knows how to power up those PM domains.

I'm a bit reluctant to split responsibility across two drivers: a
platform-specific
one handling PM domains with non-cpu devices, and a generic one handling
PM domains with cpu-devices.

Perhaps the generic one can be optional, and provide helpers for common
CPU operations? Then the platform-specific driver can handle all PM domains,
and delegate to the generic CPU helpers where appropriate.

Does that make sense?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



More information about the linux-arm-kernel mailing list