[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