| Message ID | 20220125171809.1273269-3-daniel.lezcano@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | powercap/drivers/dtpm: Create the dtpm hierarchy | expand |
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 >
On 25/01/2022 18:36, Ulf Hansson wrote: > 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. Thanks for your time to review the code
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