| Message ID | 20210517130823.796963-6-anup.patel@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V CPU Idle Support | expand |
On Mon, 17 May 2021 at 15:10, Anup Patel <anup.patel@wdc.com> wrote: > > The generic power domain related code in PSCI domain driver is largely > independent of PSCI and can be shared with RISC-V SBI domain driver > hence we factor-out this code into dt_idle_genpd.c and dt_idle_genpd.h. > > Signed-off-by: Anup Patel <anup.patel@wdc.com> This is clearly a big step in the right direction. Just a couple minor things, see more below. Note that, I have a couple of patches in the pipe for the cpuidle-psci-domain driver (not ready to be posted). I need a couple of more days to confirm this restructuring still makes sense beyond these potential new changes. I will let you know as soon as I can with the outcome. [...] > diff --git a/drivers/cpuidle/dt_idle_genpd.c b/drivers/cpuidle/dt_idle_genpd.c I think it would be a good idea to add a new section for this to the MAINTAINERS file. Perhaps a "DT IDLE DOMAIN" section? Or perhaps you have another idea? In any case, I am happy to continue with maintenance of this code, even in the new restructured form. > new file mode 100644 > index 000000000000..5a901773db60 > --- /dev/null > +++ b/drivers/cpuidle/dt_idle_genpd.c > @@ -0,0 +1,182 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * PM domains for CPUs via genpd. > + * > + * Copyright (C) 2019 Linaro Ltd. > + * Author: Ulf Hansson <ulf.hansson@linaro.org> > + * > + * Copyright (c) 2021 Western Digital Corporation or its affiliates. > + */ > + > +#define pr_fmt(fmt) "dt-idle-genpd: " fmt > + > +#include <linux/cpu.h> > +#include <linux/device.h> > +#include <linux/kernel.h> > +#include <linux/pm_domain.h> > +#include <linux/pm_runtime.h> > +#include <linux/slab.h> > +#include <linux/string.h> > + > +#include "dt_idle_genpd.h" > + > +static int dt_pd_parse_state_nodes( > + int (*parse_state)(struct device_node *, u32 *), > + struct genpd_power_state *states, int state_count) > +{ > + int i, ret; > + u32 state, *state_buf; > + > + for (i = 0; i < state_count; i++) { > + ret = parse_state(to_of_node(states[i].fwnode), &state); > + if (ret) > + goto free_state; > + > + state_buf = kmalloc(sizeof(u32), GFP_KERNEL); > + if (!state_buf) { > + ret = -ENOMEM; > + goto free_state; > + } > + *state_buf = state; > + states[i].data = state_buf; > + } > + > + return 0; > + > +free_state: > + i--; > + for (; i >= 0; i--) > + kfree(states[i].data); > + return ret; > +} > + > +static int dt_pd_parse_states(struct device_node *np, > + int (*parse_state)(struct device_node *, u32 *), > + struct genpd_power_state **states, > + int *state_count) > +{ > + int ret; > + > + /* Parse the domain idle states. */ > + ret = of_genpd_parse_idle_states(np, states, state_count); > + if (ret) > + return ret; > + > + /* Fill out the dt specifics for each found state. */ > + ret = dt_pd_parse_state_nodes(parse_state, *states, *state_count); > + if (ret) > + kfree(*states); > + > + return ret; > +} > + > +static void dt_pd_free_states(struct genpd_power_state *states, > + unsigned int state_count) > +{ > + int i; > + > + for (i = 0; i < state_count; i++) > + kfree(states[i].data); > + kfree(states); > +} > + > +void dt_pd_free(struct generic_pm_domain *pd) > +{ > + dt_pd_free_states(pd->states, pd->state_count); > + kfree(pd->name); > + kfree(pd); > +} > +EXPORT_SYMBOL_GPL(dt_pd_free); > + > +struct generic_pm_domain *dt_pd_alloc(struct device_node *np, > + int (*parse_state)(struct device_node *, u32 *)) > +{ > + struct generic_pm_domain *pd; > + struct genpd_power_state *states = NULL; > + int ret, state_count = 0; > + > + pd = kzalloc(sizeof(*pd), GFP_KERNEL); > + if (!pd) > + goto out; > + > + pd->name = kasprintf(GFP_KERNEL, "%pOF", np); > + if (!pd->name) > + goto free_pd; > + > + /* > + * Parse the domain idle states and let genpd manage the state selection > + * for those being compatible with "domain-idle-state". > + */ > + ret = dt_pd_parse_states(np, parse_state, &states, &state_count); > + if (ret) > + goto free_name; > + > + pd->free_states = dt_pd_free_states; > + pd->name = kbasename(pd->name); > + pd->states = states; > + pd->state_count = state_count; > + > + pr_debug("alloc PM domain %s\n", pd->name); > + return pd; > + > +free_name: > + kfree(pd->name); > +free_pd: > + kfree(pd); > +out: > + pr_err("failed to alloc PM domain %pOF\n", np); > + return NULL; > +} > +EXPORT_SYMBOL_GPL(dt_pd_alloc); > + > +int dt_pd_init_topology(struct device_node *np) > +{ > + struct device_node *node; > + struct of_phandle_args child, parent; > + int ret; > + > + for_each_child_of_node(np, node) { > + if (of_parse_phandle_with_args(node, "power-domains", > + "#power-domain-cells", 0, &parent)) > + continue; > + > + child.np = node; > + child.args_count = 0; > + ret = of_genpd_add_subdomain(&parent, &child); > + of_node_put(parent.np); > + if (ret) { > + of_node_put(node); > + return ret; > + } > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(dt_pd_init_topology); May I suggest that we stick to dt_idle_* as the prefix for all of the exported functions in this file. Static functions can just skip the prefix altogether. > + > +struct device *dt_idle_genpd_attach_cpu(int cpu, const char *name) > +{ > + struct device *dev; > + > + dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), name); > + if (IS_ERR_OR_NULL(dev)) > + return dev; > + > + pm_runtime_irq_safe(dev); > + if (cpu_online(cpu)) > + pm_runtime_get_sync(dev); > + > + dev_pm_syscore_device(dev, true); > + > + return dev; > +} > +EXPORT_SYMBOL_GPL(dt_idle_genpd_attach_cpu); > + > +void dt_idle_genpd_detach_cpu(struct device *dev) > +{ > + if (IS_ERR_OR_NULL(dev)) > + return; > + > + dev_pm_domain_detach(dev, false); > +} > +EXPORT_SYMBOL_GPL(dt_idle_genpd_detach_cpu); Again, a minor comment on the naming of the functions. How about skipping "genpd" in the prefix, thus just dt_idle_attach|detach_cpu()? [...] Kind regards Uffe
On Mon, May 24, 2021 at 11:31 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Mon, 17 May 2021 at 15:10, Anup Patel <anup.patel@wdc.com> wrote: > > > > The generic power domain related code in PSCI domain driver is largely > > independent of PSCI and can be shared with RISC-V SBI domain driver > > hence we factor-out this code into dt_idle_genpd.c and dt_idle_genpd.h. > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > This is clearly a big step in the right direction. Just a couple minor > things, see more below. > > Note that, I have a couple of patches in the pipe for the > cpuidle-psci-domain driver (not ready to be posted). I need a couple > of more days to confirm this restructuring still makes sense beyond > these potential new changes. I will let you know as soon as I can with > the outcome. Sure, I will wait for more comments from you. I was thinking of sending next revision of patches sometime next week with the renaming of function names which you suggested. > > [...] > > > diff --git a/drivers/cpuidle/dt_idle_genpd.c b/drivers/cpuidle/dt_idle_genpd.c > > I think it would be a good idea to add a new section for this to the > MAINTAINERS file. Perhaps a "DT IDLE DOMAIN" section? Or perhaps you > have another idea? > > In any case, I am happy to continue with maintenance of this code, > even in the new restructured form. Yes, a separate "DT IDLE DOMAIN" section in MAINTAINERS file sounds good to me. Anyway the dt_idle_genpd is factored-out code from cpuidle-psci-domain.c so I suggest you to maintain dt_idle_genpd as well. Do you want me to add a "DT IDLE DOMAIN" section in the MAINTAINERS file as part of this patch ?? > > > new file mode 100644 > > index 000000000000..5a901773db60 > > --- /dev/null > > +++ b/drivers/cpuidle/dt_idle_genpd.c > > @@ -0,0 +1,182 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * PM domains for CPUs via genpd. > > + * > > + * Copyright (C) 2019 Linaro Ltd. > > + * Author: Ulf Hansson <ulf.hansson@linaro.org> > > + * > > + * Copyright (c) 2021 Western Digital Corporation or its affiliates. > > + */ > > + > > +#define pr_fmt(fmt) "dt-idle-genpd: " fmt > > + > > +#include <linux/cpu.h> > > +#include <linux/device.h> > > +#include <linux/kernel.h> > > +#include <linux/pm_domain.h> > > +#include <linux/pm_runtime.h> > > +#include <linux/slab.h> > > +#include <linux/string.h> > > + > > +#include "dt_idle_genpd.h" > > + > > +static int dt_pd_parse_state_nodes( > > + int (*parse_state)(struct device_node *, u32 *), > > + struct genpd_power_state *states, int state_count) > > +{ > > + int i, ret; > > + u32 state, *state_buf; > > + > > + for (i = 0; i < state_count; i++) { > > + ret = parse_state(to_of_node(states[i].fwnode), &state); > > + if (ret) > > + goto free_state; > > + > > + state_buf = kmalloc(sizeof(u32), GFP_KERNEL); > > + if (!state_buf) { > > + ret = -ENOMEM; > > + goto free_state; > > + } > > + *state_buf = state; > > + states[i].data = state_buf; > > + } > > + > > + return 0; > > + > > +free_state: > > + i--; > > + for (; i >= 0; i--) > > + kfree(states[i].data); > > + return ret; > > +} > > + > > +static int dt_pd_parse_states(struct device_node *np, > > + int (*parse_state)(struct device_node *, u32 *), > > + struct genpd_power_state **states, > > + int *state_count) > > +{ > > + int ret; > > + > > + /* Parse the domain idle states. */ > > + ret = of_genpd_parse_idle_states(np, states, state_count); > > + if (ret) > > + return ret; > > + > > + /* Fill out the dt specifics for each found state. */ > > + ret = dt_pd_parse_state_nodes(parse_state, *states, *state_count); > > + if (ret) > > + kfree(*states); > > + > > + return ret; > > +} > > + > > +static void dt_pd_free_states(struct genpd_power_state *states, > > + unsigned int state_count) > > +{ > > + int i; > > + > > + for (i = 0; i < state_count; i++) > > + kfree(states[i].data); > > + kfree(states); > > +} > > + > > +void dt_pd_free(struct generic_pm_domain *pd) > > +{ > > + dt_pd_free_states(pd->states, pd->state_count); > > + kfree(pd->name); > > + kfree(pd); > > +} > > +EXPORT_SYMBOL_GPL(dt_pd_free); > > + > > +struct generic_pm_domain *dt_pd_alloc(struct device_node *np, > > + int (*parse_state)(struct device_node *, u32 *)) > > +{ > > + struct generic_pm_domain *pd; > > + struct genpd_power_state *states = NULL; > > + int ret, state_count = 0; > > + > > + pd = kzalloc(sizeof(*pd), GFP_KERNEL); > > + if (!pd) > > + goto out; > > + > > + pd->name = kasprintf(GFP_KERNEL, "%pOF", np); > > + if (!pd->name) > > + goto free_pd; > > + > > + /* > > + * Parse the domain idle states and let genpd manage the state selection > > + * for those being compatible with "domain-idle-state". > > + */ > > + ret = dt_pd_parse_states(np, parse_state, &states, &state_count); > > + if (ret) > > + goto free_name; > > + > > + pd->free_states = dt_pd_free_states; > > + pd->name = kbasename(pd->name); > > + pd->states = states; > > + pd->state_count = state_count; > > + > > + pr_debug("alloc PM domain %s\n", pd->name); > > + return pd; > > + > > +free_name: > > + kfree(pd->name); > > +free_pd: > > + kfree(pd); > > +out: > > + pr_err("failed to alloc PM domain %pOF\n", np); > > + return NULL; > > +} > > +EXPORT_SYMBOL_GPL(dt_pd_alloc); > > + > > +int dt_pd_init_topology(struct device_node *np) > > +{ > > + struct device_node *node; > > + struct of_phandle_args child, parent; > > + int ret; > > + > > + for_each_child_of_node(np, node) { > > + if (of_parse_phandle_with_args(node, "power-domains", > > + "#power-domain-cells", 0, &parent)) > > + continue; > > + > > + child.np = node; > > + child.args_count = 0; > > + ret = of_genpd_add_subdomain(&parent, &child); > > + of_node_put(parent.np); > > + if (ret) { > > + of_node_put(node); > > + return ret; > > + } > > + } > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(dt_pd_init_topology); > > May I suggest that we stick to dt_idle_* as the prefix for all of the > exported functions in this file. Static functions can just skip the > prefix altogether. Sure, I will update the function names like you suggested in next patch revision. > > > + > > +struct device *dt_idle_genpd_attach_cpu(int cpu, const char *name) > > +{ > > + struct device *dev; > > + > > + dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), name); > > + if (IS_ERR_OR_NULL(dev)) > > + return dev; > > + > > + pm_runtime_irq_safe(dev); > > + if (cpu_online(cpu)) > > + pm_runtime_get_sync(dev); > > + > > + dev_pm_syscore_device(dev, true); > > + > > + return dev; > > +} > > +EXPORT_SYMBOL_GPL(dt_idle_genpd_attach_cpu); > > + > > +void dt_idle_genpd_detach_cpu(struct device *dev) > > +{ > > + if (IS_ERR_OR_NULL(dev)) > > + return; > > + > > + dev_pm_domain_detach(dev, false); > > +} > > +EXPORT_SYMBOL_GPL(dt_idle_genpd_detach_cpu); > > Again, a minor comment on the naming of the functions. How about > skipping "genpd" in the prefix, thus just dt_idle_attach|detach_cpu()? Sure, I will update. > > [...] > > Kind regards > Uffe Regards, Anup
On Tue, 25 May 2021 at 07:39, Anup Patel <anup@brainfault.org> wrote: > > On Mon, May 24, 2021 at 11:31 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On Mon, 17 May 2021 at 15:10, Anup Patel <anup.patel@wdc.com> wrote: > > > > > > The generic power domain related code in PSCI domain driver is largely > > > independent of PSCI and can be shared with RISC-V SBI domain driver > > > hence we factor-out this code into dt_idle_genpd.c and dt_idle_genpd.h. > > > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > > > This is clearly a big step in the right direction. Just a couple minor > > things, see more below. > > > > Note that, I have a couple of patches in the pipe for the > > cpuidle-psci-domain driver (not ready to be posted). I need a couple > > of more days to confirm this restructuring still makes sense beyond > > these potential new changes. I will let you know as soon as I can with > > the outcome. > > Sure, I will wait for more comments from you. I was thinking of sending > next revision of patches sometime next week with the renaming of > function names which you suggested. Sounds good, that allows me a few more days this week. > > > > > [...] > > > > > diff --git a/drivers/cpuidle/dt_idle_genpd.c b/drivers/cpuidle/dt_idle_genpd.c > > > > I think it would be a good idea to add a new section for this to the > > MAINTAINERS file. Perhaps a "DT IDLE DOMAIN" section? Or perhaps you > > have another idea? > > > > In any case, I am happy to continue with maintenance of this code, > > even in the new restructured form. > > Yes, a separate "DT IDLE DOMAIN" section in MAINTAINERS file > sounds good to me. > > Anyway the dt_idle_genpd is factored-out code from cpuidle-psci-domain.c > so I suggest you to maintain dt_idle_genpd as well. > > Do you want me to add a "DT IDLE DOMAIN" section in the > MAINTAINERS file as part of this patch ?? Yeah, that works for me. Perhaps extend it to .. PM DOMAIN though. [...] Kind regards Uffe
On Tue, 25 May 2021 at 11:05, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Tue, 25 May 2021 at 07:39, Anup Patel <anup@brainfault.org> wrote: > > > > On Mon, May 24, 2021 at 11:31 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > > > On Mon, 17 May 2021 at 15:10, Anup Patel <anup.patel@wdc.com> wrote: > > > > > > > > The generic power domain related code in PSCI domain driver is largely > > > > independent of PSCI and can be shared with RISC-V SBI domain driver > > > > hence we factor-out this code into dt_idle_genpd.c and dt_idle_genpd.h. > > > > > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > > > > > This is clearly a big step in the right direction. Just a couple minor > > > things, see more below. > > > > > > Note that, I have a couple of patches in the pipe for the > > > cpuidle-psci-domain driver (not ready to be posted). I need a couple > > > of more days to confirm this restructuring still makes sense beyond > > > these potential new changes. I will let you know as soon as I can with > > > the outcome. > > > > Sure, I will wait for more comments from you. I was thinking of sending > > next revision of patches sometime next week with the renaming of > > function names which you suggested. > > Sounds good, that allows me a few more days this week. I don't have any further comments at this point. It looks good to me, but let me have a quick review on the next version before I provide my ack. [...] Kind regards Uffe
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig index c0aeedd66f02..f1afe7ab6b54 100644 --- a/drivers/cpuidle/Kconfig +++ b/drivers/cpuidle/Kconfig @@ -47,6 +47,10 @@ config CPU_IDLE_GOV_HALTPOLL config DT_IDLE_STATES bool +config DT_IDLE_GENPD + depends on PM_GENERIC_DOMAINS_OF + bool + menu "ARM CPU Idle Drivers" depends on ARM || ARM64 source "drivers/cpuidle/Kconfig.arm" diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm index 334f83e56120..be12a9ca78f0 100644 --- a/drivers/cpuidle/Kconfig.arm +++ b/drivers/cpuidle/Kconfig.arm @@ -27,6 +27,7 @@ config ARM_PSCI_CPUIDLE_DOMAIN bool "PSCI CPU idle Domain" depends on ARM_PSCI_CPUIDLE depends on PM_GENERIC_DOMAINS_OF + select DT_IDLE_GENPD default y help Select this to enable the PSCI based CPUidle driver to use PM domains, diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile index 26bbc5e74123..11a26cef279f 100644 --- a/drivers/cpuidle/Makefile +++ b/drivers/cpuidle/Makefile @@ -6,6 +6,7 @@ obj-y += cpuidle.o driver.o governor.o sysfs.o governors/ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o obj-$(CONFIG_DT_IDLE_STATES) += dt_idle_states.o +obj-$(CONFIG_DT_IDLE_GENPD) += dt_idle_genpd.o obj-$(CONFIG_ARCH_HAS_CPU_RELAX) += poll_state.o obj-$(CONFIG_HALTPOLL_CPUIDLE) += cpuidle-haltpoll.o diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c index ff2c3f8e4668..3ceec0ee9f02 100644 --- a/drivers/cpuidle/cpuidle-psci-domain.c +++ b/drivers/cpuidle/cpuidle-psci-domain.c @@ -47,73 +47,14 @@ static int psci_pd_power_off(struct generic_pm_domain *pd) return 0; } -static int psci_pd_parse_state_nodes(struct genpd_power_state *states, - int state_count) -{ - int i, ret; - u32 psci_state, *psci_state_buf; - - for (i = 0; i < state_count; i++) { - ret = psci_dt_parse_state_node(to_of_node(states[i].fwnode), - &psci_state); - if (ret) - goto free_state; - - psci_state_buf = kmalloc(sizeof(u32), GFP_KERNEL); - if (!psci_state_buf) { - ret = -ENOMEM; - goto free_state; - } - *psci_state_buf = psci_state; - states[i].data = psci_state_buf; - } - - return 0; - -free_state: - i--; - for (; i >= 0; i--) - kfree(states[i].data); - return ret; -} - -static int psci_pd_parse_states(struct device_node *np, - struct genpd_power_state **states, int *state_count) -{ - int ret; - - /* Parse the domain idle states. */ - ret = of_genpd_parse_idle_states(np, states, state_count); - if (ret) - return ret; - - /* Fill out the PSCI specifics for each found state. */ - ret = psci_pd_parse_state_nodes(*states, *state_count); - if (ret) - kfree(*states); - - return ret; -} - -static void psci_pd_free_states(struct genpd_power_state *states, - unsigned int state_count) -{ - int i; - - for (i = 0; i < state_count; i++) - kfree(states[i].data); - kfree(states); -} - static int psci_pd_init(struct device_node *np, bool use_osi) { struct generic_pm_domain *pd; struct psci_pd_provider *pd_provider; struct dev_power_governor *pd_gov; - struct genpd_power_state *states = NULL; int ret = -ENOMEM, state_count = 0; - pd = kzalloc(sizeof(*pd), GFP_KERNEL); + pd = dt_pd_alloc(np, psci_dt_parse_state_node); if (!pd) goto out; @@ -121,22 +62,6 @@ static int psci_pd_init(struct device_node *np, bool use_osi) if (!pd_provider) goto free_pd; - pd->name = kasprintf(GFP_KERNEL, "%pOF", np); - if (!pd->name) - goto free_pd_prov; - - /* - * Parse the domain idle states and let genpd manage the state selection - * for those being compatible with "domain-idle-state". - */ - ret = psci_pd_parse_states(np, &states, &state_count); - if (ret) - goto free_name; - - pd->free_states = psci_pd_free_states; - pd->name = kbasename(pd->name); - pd->states = states; - pd->state_count = state_count; pd->flags |= GENPD_FLAG_IRQ_SAFE | GENPD_FLAG_CPU_DOMAIN; /* Allow power off when OSI has been successfully enabled. */ @@ -149,10 +74,8 @@ static int psci_pd_init(struct device_node *np, bool use_osi) pd_gov = state_count > 0 ? &pm_domain_cpu_gov : NULL; ret = pm_genpd_init(pd, pd_gov, false); - if (ret) { - psci_pd_free_states(states, state_count); - goto free_name; - } + if (ret) + goto free_pd_prov; ret = of_genpd_add_provider_simple(np, pd); if (ret) @@ -166,12 +89,10 @@ static int psci_pd_init(struct device_node *np, bool use_osi) remove_pd: pm_genpd_remove(pd); -free_name: - kfree(pd->name); free_pd_prov: kfree(pd_provider); free_pd: - kfree(pd); + dt_pd_free(pd); out: pr_err("failed to init PM domain ret=%d %pOF\n", ret, np); return ret; @@ -195,30 +116,6 @@ static void psci_pd_remove(void) } } -static int psci_pd_init_topology(struct device_node *np) -{ - struct device_node *node; - struct of_phandle_args child, parent; - int ret; - - for_each_child_of_node(np, node) { - if (of_parse_phandle_with_args(node, "power-domains", - "#power-domain-cells", 0, &parent)) - continue; - - child.np = node; - child.args_count = 0; - ret = of_genpd_add_subdomain(&parent, &child); - of_node_put(parent.np); - if (ret) { - of_node_put(node); - return ret; - } - } - - return 0; -} - static bool psci_pd_try_set_osi_mode(void) { int ret; @@ -282,7 +179,7 @@ static int psci_cpuidle_domain_probe(struct platform_device *pdev) goto no_pd; /* Link genpd masters/subdomains to model the CPU topology. */ - ret = psci_pd_init_topology(np); + ret = dt_pd_init_topology(np); if (ret) goto remove_pd; @@ -314,28 +211,3 @@ static int __init psci_idle_init_domains(void) return platform_driver_register(&psci_cpuidle_domain_driver); } subsys_initcall(psci_idle_init_domains); - -struct device *psci_dt_attach_cpu(int cpu) -{ - struct device *dev; - - dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), "psci"); - if (IS_ERR_OR_NULL(dev)) - return dev; - - pm_runtime_irq_safe(dev); - if (cpu_online(cpu)) - pm_runtime_get_sync(dev); - - dev_pm_syscore_device(dev, true); - - return dev; -} - -void psci_dt_detach_cpu(struct device *dev) -{ - if (IS_ERR_OR_NULL(dev)) - return; - - dev_pm_domain_detach(dev, false); -} diff --git a/drivers/cpuidle/cpuidle-psci.h b/drivers/cpuidle/cpuidle-psci.h index d8e925e84c27..70de1e3c00af 100644 --- a/drivers/cpuidle/cpuidle-psci.h +++ b/drivers/cpuidle/cpuidle-psci.h @@ -10,8 +10,19 @@ void psci_set_domain_state(u32 state); int psci_dt_parse_state_node(struct device_node *np, u32 *state); #ifdef CONFIG_ARM_PSCI_CPUIDLE_DOMAIN -struct device *psci_dt_attach_cpu(int cpu); -void psci_dt_detach_cpu(struct device *dev); + +#include "dt_idle_genpd.h" + +static inline struct device *psci_dt_attach_cpu(int cpu) +{ + return dt_idle_genpd_attach_cpu(cpu, "psci"); +} + +static inline void psci_dt_detach_cpu(struct device *dev) +{ + dt_idle_genpd_detach_cpu(dev); +} + #else static inline struct device *psci_dt_attach_cpu(int cpu) { return NULL; } static inline void psci_dt_detach_cpu(struct device *dev) { } diff --git a/drivers/cpuidle/dt_idle_genpd.c b/drivers/cpuidle/dt_idle_genpd.c new file mode 100644 index 000000000000..5a901773db60 --- /dev/null +++ b/drivers/cpuidle/dt_idle_genpd.c @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * PM domains for CPUs via genpd. + * + * Copyright (C) 2019 Linaro Ltd. + * Author: Ulf Hansson <ulf.hansson@linaro.org> + * + * Copyright (c) 2021 Western Digital Corporation or its affiliates. + */ + +#define pr_fmt(fmt) "dt-idle-genpd: " fmt + +#include <linux/cpu.h> +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/pm_domain.h> +#include <linux/pm_runtime.h> +#include <linux/slab.h> +#include <linux/string.h> + +#include "dt_idle_genpd.h" + +static int dt_pd_parse_state_nodes( + int (*parse_state)(struct device_node *, u32 *), + struct genpd_power_state *states, int state_count) +{ + int i, ret; + u32 state, *state_buf; + + for (i = 0; i < state_count; i++) { + ret = parse_state(to_of_node(states[i].fwnode), &state); + if (ret) + goto free_state; + + state_buf = kmalloc(sizeof(u32), GFP_KERNEL); + if (!state_buf) { + ret = -ENOMEM; + goto free_state; + } + *state_buf = state; + states[i].data = state_buf; + } + + return 0; + +free_state: + i--; + for (; i >= 0; i--) + kfree(states[i].data); + return ret; +} + +static int dt_pd_parse_states(struct device_node *np, + int (*parse_state)(struct device_node *, u32 *), + struct genpd_power_state **states, + int *state_count) +{ + int ret; + + /* Parse the domain idle states. */ + ret = of_genpd_parse_idle_states(np, states, state_count); + if (ret) + return ret; + + /* Fill out the dt specifics for each found state. */ + ret = dt_pd_parse_state_nodes(parse_state, *states, *state_count); + if (ret) + kfree(*states); + + return ret; +} + +static void dt_pd_free_states(struct genpd_power_state *states, + unsigned int state_count) +{ + int i; + + for (i = 0; i < state_count; i++) + kfree(states[i].data); + kfree(states); +} + +void dt_pd_free(struct generic_pm_domain *pd) +{ + dt_pd_free_states(pd->states, pd->state_count); + kfree(pd->name); + kfree(pd); +} +EXPORT_SYMBOL_GPL(dt_pd_free); + +struct generic_pm_domain *dt_pd_alloc(struct device_node *np, + int (*parse_state)(struct device_node *, u32 *)) +{ + struct generic_pm_domain *pd; + struct genpd_power_state *states = NULL; + int ret, state_count = 0; + + pd = kzalloc(sizeof(*pd), GFP_KERNEL); + if (!pd) + goto out; + + pd->name = kasprintf(GFP_KERNEL, "%pOF", np); + if (!pd->name) + goto free_pd; + + /* + * Parse the domain idle states and let genpd manage the state selection + * for those being compatible with "domain-idle-state". + */ + ret = dt_pd_parse_states(np, parse_state, &states, &state_count); + if (ret) + goto free_name; + + pd->free_states = dt_pd_free_states; + pd->name = kbasename(pd->name); + pd->states = states; + pd->state_count = state_count; + + pr_debug("alloc PM domain %s\n", pd->name); + return pd; + +free_name: + kfree(pd->name); +free_pd: + kfree(pd); +out: + pr_err("failed to alloc PM domain %pOF\n", np); + return NULL; +} +EXPORT_SYMBOL_GPL(dt_pd_alloc); + +int dt_pd_init_topology(struct device_node *np) +{ + struct device_node *node; + struct of_phandle_args child, parent; + int ret; + + for_each_child_of_node(np, node) { + if (of_parse_phandle_with_args(node, "power-domains", + "#power-domain-cells", 0, &parent)) + continue; + + child.np = node; + child.args_count = 0; + ret = of_genpd_add_subdomain(&parent, &child); + of_node_put(parent.np); + if (ret) { + of_node_put(node); + return ret; + } + } + + return 0; +} +EXPORT_SYMBOL_GPL(dt_pd_init_topology); + +struct device *dt_idle_genpd_attach_cpu(int cpu, const char *name) +{ + struct device *dev; + + dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), name); + if (IS_ERR_OR_NULL(dev)) + return dev; + + pm_runtime_irq_safe(dev); + if (cpu_online(cpu)) + pm_runtime_get_sync(dev); + + dev_pm_syscore_device(dev, true); + + return dev; +} +EXPORT_SYMBOL_GPL(dt_idle_genpd_attach_cpu); + +void dt_idle_genpd_detach_cpu(struct device *dev) +{ + if (IS_ERR_OR_NULL(dev)) + return; + + dev_pm_domain_detach(dev, false); +} +EXPORT_SYMBOL_GPL(dt_idle_genpd_detach_cpu); diff --git a/drivers/cpuidle/dt_idle_genpd.h b/drivers/cpuidle/dt_idle_genpd.h new file mode 100644 index 000000000000..23ebf6050e74 --- /dev/null +++ b/drivers/cpuidle/dt_idle_genpd.h @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __DT_IDLE_GENPD +#define __DT_IDLE_GENPD + +struct device_node; +struct generic_pm_domain; + +#ifdef CONFIG_DT_IDLE_GENPD + +void dt_pd_free(struct generic_pm_domain *pd); + +struct generic_pm_domain *dt_pd_alloc(struct device_node *np, + int (*parse_state)(struct device_node *, u32 *)); + +int dt_pd_init_topology(struct device_node *np); + +struct device *dt_idle_genpd_attach_cpu(int cpu, const char *name); + +void dt_idle_genpd_detach_cpu(struct device *dev); + +#else + +static inline void dt_pd_free(struct generic_pm_domain *pd) +{ +} + +static inline struct generic_pm_domain *dt_pd_alloc(struct device_node *np, + int (*parse_state)(struct device_node *, u32 *)); +{ + return NULL; +} + +static inline int dt_pd_init_topology(struct device_node *np) +{ + return 0; +} + +static inline struct device *dt_idle_genpd_attach_cpu(int cpu, + const char *name) +{ + return NULL; +} + +static inline void dt_idle_genpd_detach_cpu(struct device *dev) +{ +} + +#endif + +#endif
The generic power domain related code in PSCI domain driver is largely independent of PSCI and can be shared with RISC-V SBI domain driver hence we factor-out this code into dt_idle_genpd.c and dt_idle_genpd.h. Signed-off-by: Anup Patel <anup.patel@wdc.com> --- drivers/cpuidle/Kconfig | 4 + drivers/cpuidle/Kconfig.arm | 1 + drivers/cpuidle/Makefile | 1 + drivers/cpuidle/cpuidle-psci-domain.c | 138 +------------------ drivers/cpuidle/cpuidle-psci.h | 15 ++- drivers/cpuidle/dt_idle_genpd.c | 182 ++++++++++++++++++++++++++ drivers/cpuidle/dt_idle_genpd.h | 50 +++++++ 7 files changed, 256 insertions(+), 135 deletions(-) create mode 100644 drivers/cpuidle/dt_idle_genpd.c create mode 100644 drivers/cpuidle/dt_idle_genpd.h