Messages in this thread | | | From | Ulf Hansson <> | Date | Tue, 25 Jan 2022 18:36:49 +0100 | Subject | Re: [PATCH v7 2/5] powercap/drivers/dtpm: Add hierarchy creation |
| |
On Tue, 25 Jan 2022 at 18:18, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > The DTPM framework is available but without a way to configure it. > > This change provides a way to create a hierarchy of DTPM node where > the power consumption reflects the sum of the children's power > consumption. > > It is up to the platform to specify an array of dtpm nodes where each > element has a pointer to its parent, except the top most one. The type > of the node gives the indication of which initialization callback to > call. At this time, we can create a virtual node, where its purpose is > to be a parent in the hierarchy, and a DT node where the name > describes its path. > > In order to ensure a nice self-encapsulation, the DTPM subsys array > contains a couple of initialization functions, one to setup the DTPM > backend and one to initialize it up. With this approach, the DTPM > framework has a very few material to export. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Yes, this looks good to me now. Thanks for adopting my suggestions.
Kind regards Uffe
> --- > drivers/powercap/Kconfig | 1 + > drivers/powercap/dtpm.c | 190 ++++++++++++++++++++++++++++++++++++++- > include/linux/dtpm.h | 15 ++++ > 3 files changed, 203 insertions(+), 3 deletions(-) > > diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig > index 8242e8c5ed77..b1ca339957e3 100644 > --- a/drivers/powercap/Kconfig > +++ b/drivers/powercap/Kconfig > @@ -46,6 +46,7 @@ config IDLE_INJECT > > config DTPM > bool "Power capping for Dynamic Thermal Power Management (EXPERIMENTAL)" > + depends on OF > help > This enables support for the power capping for the dynamic > thermal power management userspace engine. > diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c > index 0e5c93443c70..414826a1509b 100644 > --- a/drivers/powercap/dtpm.c > +++ b/drivers/powercap/dtpm.c > @@ -23,6 +23,7 @@ > #include <linux/powercap.h> > #include <linux/slab.h> > #include <linux/mutex.h> > +#include <linux/of.h> > > #include "dtpm_subsys.h" > > @@ -463,14 +464,197 @@ int dtpm_register(const char *name, struct dtpm *dtpm, struct dtpm *parent) > return 0; > } > > -static int __init init_dtpm(void) > +static struct dtpm *dtpm_setup_virtual(const struct dtpm_node *hierarchy, > + struct dtpm *parent) > { > + struct dtpm *dtpm; > + int ret; > + > + dtpm = kzalloc(sizeof(*dtpm), GFP_KERNEL); > + if (!dtpm) > + return ERR_PTR(-ENOMEM); > + dtpm_init(dtpm, NULL); > + > + ret = dtpm_register(hierarchy->name, dtpm, parent); > + if (ret) { > + pr_err("Failed to register dtpm node '%s': %d\n", > + hierarchy->name, ret); > + kfree(dtpm); > + return ERR_PTR(ret); > + } > + > + return dtpm; > +} > + > +static struct dtpm *dtpm_setup_dt(const struct dtpm_node *hierarchy, > + struct dtpm *parent) > +{ > + struct device_node *np; > + int i, ret; > + > + np = of_find_node_by_path(hierarchy->name); > + if (!np) { > + pr_err("Failed to find '%s'\n", hierarchy->name); > + return ERR_PTR(-ENXIO); > + } > + > + for (i = 0; i < ARRAY_SIZE(dtpm_subsys); i++) { > + > + if (!dtpm_subsys[i]->setup) > + continue; > + > + ret = dtpm_subsys[i]->setup(parent, np); > + if (ret) { > + pr_err("Failed to setup '%s': %d\n", dtpm_subsys[i]->name, ret); > + of_node_put(np); > + return ERR_PTR(ret); > + } > + } > + > + of_node_put(np); > + > + /* > + * By returning a NULL pointer, we let know the caller there > + * is no child for us as we are a leaf of the tree > + */ > + return NULL; > +} > + > +typedef struct dtpm * (*dtpm_node_callback_t)(const struct dtpm_node *, struct dtpm *); > + > +dtpm_node_callback_t dtpm_node_callback[] = { > + [DTPM_NODE_VIRTUAL] = dtpm_setup_virtual, > + [DTPM_NODE_DT] = dtpm_setup_dt, > +}; > + > +static int dtpm_for_each_child(const struct dtpm_node *hierarchy, > + const struct dtpm_node *it, struct dtpm *parent) > +{ > + struct dtpm *dtpm; > + int i, ret; > + > + for (i = 0; hierarchy[i].name; i++) { > + > + if (hierarchy[i].parent != it) > + continue; > + > + dtpm = dtpm_node_callback[hierarchy[i].type](&hierarchy[i], parent); > + > + /* > + * A NULL pointer means there is no children, hence we > + * continue without going deeper in the recursivity. > + */ > + if (!dtpm) > + continue; > + > + /* > + * There are multiple reasons why the callback could > + * fail. The generic glue is abstracting the backend > + * and therefore it is not possible to report back or > + * take a decision based on the error. In any case, > + * if this call fails, it is not critical in the > + * hierarchy creation, we can assume the underlying > + * service is not found, so we continue without this > + * branch in the tree but with a warning to log the > + * information the node was not created. > + */ > + if (IS_ERR(dtpm)) { > + pr_warn("Failed to create '%s' in the hierarchy\n", > + hierarchy[i].name); > + continue; > + } > + > + ret = dtpm_for_each_child(hierarchy, &hierarchy[i], dtpm); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +/** > + * dtpm_create_hierarchy - Create the dtpm hierarchy > + * @hierarchy: An array of struct dtpm_node describing the hierarchy > + * > + * The function is called by the platform specific code with the > + * description of the different node in the hierarchy. It creates the > + * tree in the sysfs filesystem under the powercap dtpm entry. > + * > + * The expected tree has the format: > + * > + * struct dtpm_node hierarchy[] = { > + * [0] { .name = "topmost", type = DTPM_NODE_VIRTUAL }, > + * [1] { .name = "package", .type = DTPM_NODE_VIRTUAL, .parent = &hierarchy[0] }, > + * [2] { .name = "/cpus/cpu0", .type = DTPM_NODE_DT, .parent = &hierarchy[1] }, > + * [3] { .name = "/cpus/cpu1", .type = DTPM_NODE_DT, .parent = &hierarchy[1] }, > + * [4] { .name = "/cpus/cpu2", .type = DTPM_NODE_DT, .parent = &hierarchy[1] }, > + * [5] { .name = "/cpus/cpu3", .type = DTPM_NODE_DT, .parent = &hierarchy[1] }, > + * [6] { } > + * }; > + * > + * The last element is always an empty one and marks the end of the > + * array. > + * > + * Return: zero on success, a negative value in case of error. Errors > + * are reported back from the underlying functions. > + */ > +int dtpm_create_hierarchy(struct of_device_id *dtpm_match_table) > +{ > + const struct of_device_id *match; > + const struct dtpm_node *hierarchy; > + struct device_node *np; > + int i, ret; > + > + if (pct) > + return -EBUSY; > + > pct = powercap_register_control_type(NULL, "dtpm", NULL); > if (IS_ERR(pct)) { > pr_err("Failed to register control type\n"); > - return PTR_ERR(pct); > + ret = PTR_ERR(pct); > + goto out_pct; > + } > + > + ret = -ENODEV; > + np = of_find_node_by_path("/"); > + if (!np) > + goto out_err; > + > + match = of_match_node(dtpm_match_table, np); > + > + of_node_put(np); > + > + if (!match) > + goto out_err; > + > + hierarchy = match->data; > + if (!hierarchy) { > + ret = -EFAULT; > + goto out_err; > + } > + > + ret = dtpm_for_each_child(hierarchy, NULL, NULL); > + if (ret) > + goto out_err; > + > + for (i = 0; i < ARRAY_SIZE(dtpm_subsys); i++) { > + > + if (!dtpm_subsys[i]->init) > + continue; > + > + ret = dtpm_subsys[i]->init(); > + if (ret) > + pr_info("Failed to initialze '%s': %d", > + dtpm_subsys[i]->name, ret); > } > > return 0; > + > +out_err: > + powercap_unregister_control_type(pct); > +out_pct: > + pct = NULL; > + > + return ret; > } > -late_initcall(init_dtpm); > +EXPORT_SYMBOL_GPL(dtpm_create_hierarchy); > diff --git a/include/linux/dtpm.h b/include/linux/dtpm.h > index 506048158a50..f7a25c70dd4c 100644 > --- a/include/linux/dtpm.h > +++ b/include/linux/dtpm.h > @@ -32,9 +32,23 @@ struct dtpm_ops { > void (*release)(struct dtpm *); > }; > > +struct device_node; > + > struct dtpm_subsys_ops { > const char *name; > int (*init)(void); > + int (*setup)(struct dtpm *, struct device_node *); > +}; > + > +enum DTPM_NODE_TYPE { > + DTPM_NODE_VIRTUAL = 0, > + DTPM_NODE_DT, > +}; > + > +struct dtpm_node { > + enum DTPM_NODE_TYPE type; > + const char *name; > + struct dtpm_node *parent; > }; > > static inline struct dtpm *to_dtpm(struct powercap_zone *zone) > @@ -52,4 +66,5 @@ void dtpm_unregister(struct dtpm *dtpm); > > int dtpm_register(const char *name, struct dtpm *dtpm, struct dtpm *parent); > > +int dtpm_create_hierarchy(struct of_device_id *dtpm_match_table); > #endif > -- > 2.25.1 >
| |