From b55eef5226b71edf5422de246bc189da1fdc9000 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Thu, 13 Oct 2022 18:46:12 +0100 Subject: [PATCH 01/11] powercap: arm_scmi: Add SCMI Powercap based driver Add a powercap driver that, using the ARM SCMI Protocol to query the SCMI platform firmware for the list of existing Powercap domains, registers all of such discovered domains under the new 'arm-scmi' powercap control type. A new simple powercap zone and constraint is registered for all the SCMI powercap zones that are found. Reviewed-by: Lukasz Luba Signed-off-by: Cristian Marussi Acked-by: Sudeep Holla Signed-off-by: Rafael J. Wysocki --- MAINTAINERS | 1 + drivers/powercap/Kconfig | 13 + drivers/powercap/Makefile | 1 + drivers/powercap/arm_scmi_powercap.c | 509 +++++++++++++++++++++++++++ 4 files changed, 524 insertions(+) create mode 100644 drivers/powercap/arm_scmi_powercap.c diff --git a/MAINTAINERS b/MAINTAINERS index e04d944005ba..141251b562d9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -19977,6 +19977,7 @@ F: drivers/clk/clk-sc[mp]i.c F: drivers/cpufreq/sc[mp]i-cpufreq.c F: drivers/firmware/arm_scmi/ F: drivers/firmware/arm_scpi.c +F: drivers/powercap/arm_scmi_powercap.c F: drivers/regulator/scmi-regulator.c F: drivers/reset/reset-scmi.c F: include/linux/sc[mp]i_protocol.h diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig index 515e3ceb3393..90d33cd1b670 100644 --- a/drivers/powercap/Kconfig +++ b/drivers/powercap/Kconfig @@ -44,6 +44,19 @@ config IDLE_INJECT synchronously on a set of specified CPUs or alternatively on a per CPU basis. +config ARM_SCMI_POWERCAP + tristate "ARM SCMI Powercap driver" + depends on ARM_SCMI_PROTOCOL + help + This enables support for the ARM Powercap based on ARM SCMI + Powercap protocol. + + ARM SCMI Powercap protocol allows power limits to be enforced + and monitored against the SCMI Powercap domains advertised as + available by the SCMI platform firmware. + + When compiled as module it will be called arm_scmi_powercap.ko. + config DTPM bool "Power capping for Dynamic Thermal Power Management (EXPERIMENTAL)" depends on OF diff --git a/drivers/powercap/Makefile b/drivers/powercap/Makefile index 494617cdad88..4474201b4aa7 100644 --- a/drivers/powercap/Makefile +++ b/drivers/powercap/Makefile @@ -6,3 +6,4 @@ obj-$(CONFIG_POWERCAP) += powercap_sys.o obj-$(CONFIG_INTEL_RAPL_CORE) += intel_rapl_common.o obj-$(CONFIG_INTEL_RAPL) += intel_rapl_msr.o obj-$(CONFIG_IDLE_INJECT) += idle_inject.o +obj-$(CONFIG_ARM_SCMI_POWERCAP) += arm_scmi_powercap.o diff --git a/drivers/powercap/arm_scmi_powercap.c b/drivers/powercap/arm_scmi_powercap.c new file mode 100644 index 000000000000..05d0e516176a --- /dev/null +++ b/drivers/powercap/arm_scmi_powercap.c @@ -0,0 +1,509 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SCMI Powercap support. + * + * Copyright (C) 2022 ARM Ltd. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define to_scmi_powercap_zone(z) \ + container_of(z, struct scmi_powercap_zone, zone) + +static const struct scmi_powercap_proto_ops *powercap_ops; + +struct scmi_powercap_zone { + unsigned int height; + struct device *dev; + struct scmi_protocol_handle *ph; + const struct scmi_powercap_info *info; + struct scmi_powercap_zone *spzones; + struct powercap_zone zone; + struct list_head node; +}; + +struct scmi_powercap_root { + unsigned int num_zones; + struct scmi_powercap_zone *spzones; + struct list_head *registered_zones; +}; + +static struct powercap_control_type *scmi_top_pcntrl; + +static int scmi_powercap_zone_release(struct powercap_zone *pz) +{ + return 0; +} + +static int scmi_powercap_get_max_power_range_uw(struct powercap_zone *pz, + u64 *max_power_range_uw) +{ + *max_power_range_uw = U32_MAX; + return 0; +} + +static int scmi_powercap_get_power_uw(struct powercap_zone *pz, + u64 *power_uw) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + u32 avg_power, pai; + int ret; + + if (!spz->info->powercap_monitoring) + return -EINVAL; + + ret = powercap_ops->measurements_get(spz->ph, spz->info->id, &avg_power, + &pai); + if (ret) + return ret; + + *power_uw = avg_power; + if (spz->info->powercap_scale_mw) + *power_uw *= 1000; + + return 0; +} + +static const struct powercap_zone_ops zone_ops = { + .get_max_power_range_uw = scmi_powercap_get_max_power_range_uw, + .get_power_uw = scmi_powercap_get_power_uw, + .release = scmi_powercap_zone_release, +}; + +static void scmi_powercap_normalize_cap(const struct scmi_powercap_zone *spz, + u64 power_limit_uw, u32 *norm) +{ + bool scale_mw = spz->info->powercap_scale_mw; + u64 val; + + val = scale_mw ? DIV_ROUND_UP_ULL(power_limit_uw, 1000) : power_limit_uw; + /* + * This cast is lossless since here @req_power is certain to be within + * the range [min_power_cap, max_power_cap] whose bounds are assured to + * be two unsigned 32bits quantities. + */ + *norm = clamp_t(u32, val, spz->info->min_power_cap, + spz->info->max_power_cap); + *norm = rounddown(*norm, spz->info->power_cap_step); + + val = (scale_mw) ? *norm * 1000 : *norm; + if (power_limit_uw != val) + dev_dbg(spz->dev, + "Normalized %s:CAP - requested:%llu - normalized:%llu\n", + spz->info->name, power_limit_uw, val); +} + +static int scmi_powercap_set_power_limit_uw(struct powercap_zone *pz, int cid, + u64 power_uw) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + u32 norm_power; + + if (!spz->info->powercap_cap_config) + return -EINVAL; + + scmi_powercap_normalize_cap(spz, power_uw, &norm_power); + + return powercap_ops->cap_set(spz->ph, spz->info->id, norm_power, false); +} + +static int scmi_powercap_get_power_limit_uw(struct powercap_zone *pz, int cid, + u64 *power_limit_uw) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + u32 power; + int ret; + + ret = powercap_ops->cap_get(spz->ph, spz->info->id, &power); + if (ret) + return ret; + + *power_limit_uw = power; + if (spz->info->powercap_scale_mw) + *power_limit_uw *= 1000; + + return 0; +} + +static void scmi_powercap_normalize_time(const struct scmi_powercap_zone *spz, + u64 time_us, u32 *norm) +{ + /* + * This cast is lossless since here @time_us is certain to be within the + * range [min_pai, max_pai] whose bounds are assured to be two unsigned + * 32bits quantities. + */ + *norm = clamp_t(u32, time_us, spz->info->min_pai, spz->info->max_pai); + *norm = rounddown(*norm, spz->info->pai_step); + + if (time_us != *norm) + dev_dbg(spz->dev, + "Normalized %s:PAI - requested:%llu - normalized:%u\n", + spz->info->name, time_us, *norm); +} + +static int scmi_powercap_set_time_window_us(struct powercap_zone *pz, int cid, + u64 time_window_us) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + u32 norm_pai; + + if (!spz->info->powercap_pai_config) + return -EINVAL; + + scmi_powercap_normalize_time(spz, time_window_us, &norm_pai); + + return powercap_ops->pai_set(spz->ph, spz->info->id, norm_pai); +} + +static int scmi_powercap_get_time_window_us(struct powercap_zone *pz, int cid, + u64 *time_window_us) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + int ret; + u32 pai; + + ret = powercap_ops->pai_get(spz->ph, spz->info->id, &pai); + if (ret) + return ret; + + *time_window_us = pai; + + return 0; +} + +static int scmi_powercap_get_max_power_uw(struct powercap_zone *pz, int cid, + u64 *max_power_uw) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + + *max_power_uw = spz->info->max_power_cap; + if (spz->info->powercap_scale_mw) + *max_power_uw *= 1000; + + return 0; +} + +static int scmi_powercap_get_min_power_uw(struct powercap_zone *pz, int cid, + u64 *min_power_uw) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + + *min_power_uw = spz->info->min_power_cap; + if (spz->info->powercap_scale_mw) + *min_power_uw *= 1000; + + return 0; +} + +static int scmi_powercap_get_max_time_window_us(struct powercap_zone *pz, + int cid, u64 *time_window_us) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + + *time_window_us = spz->info->max_pai; + + return 0; +} + +static int scmi_powercap_get_min_time_window_us(struct powercap_zone *pz, + int cid, u64 *time_window_us) +{ + struct scmi_powercap_zone *spz = to_scmi_powercap_zone(pz); + + *time_window_us = (u64)spz->info->min_pai; + + return 0; +} + +static const char *scmi_powercap_get_name(struct powercap_zone *pz, int cid) +{ + return "SCMI power-cap"; +} + +static const struct powercap_zone_constraint_ops constraint_ops = { + .set_power_limit_uw = scmi_powercap_set_power_limit_uw, + .get_power_limit_uw = scmi_powercap_get_power_limit_uw, + .set_time_window_us = scmi_powercap_set_time_window_us, + .get_time_window_us = scmi_powercap_get_time_window_us, + .get_max_power_uw = scmi_powercap_get_max_power_uw, + .get_min_power_uw = scmi_powercap_get_min_power_uw, + .get_max_time_window_us = scmi_powercap_get_max_time_window_us, + .get_min_time_window_us = scmi_powercap_get_min_time_window_us, + .get_name = scmi_powercap_get_name, +}; + +static void scmi_powercap_unregister_all_zones(struct scmi_powercap_root *pr) +{ + int i; + + /* Un-register children zones first starting from the leaves */ + for (i = pr->num_zones - 1; i >= 0; i--) { + if (!list_empty(&pr->registered_zones[i])) { + struct scmi_powercap_zone *spz; + + list_for_each_entry(spz, &pr->registered_zones[i], node) + powercap_unregister_zone(scmi_top_pcntrl, + &spz->zone); + } + } +} + +static inline bool +scmi_powercap_is_zone_registered(struct scmi_powercap_zone *spz) +{ + return !list_empty(&spz->node); +} + +static inline unsigned int +scmi_powercap_get_zone_height(struct scmi_powercap_zone *spz) +{ + if (spz->info->parent_id == SCMI_POWERCAP_ROOT_ZONE_ID) + return 0; + + return spz->spzones[spz->info->parent_id].height + 1; +} + +static inline struct scmi_powercap_zone * +scmi_powercap_get_parent_zone(struct scmi_powercap_zone *spz) +{ + if (spz->info->parent_id == SCMI_POWERCAP_ROOT_ZONE_ID) + return NULL; + + return &spz->spzones[spz->info->parent_id]; +} + +/** + * scmi_powercap_register_zone - Register an SCMI powercap zone recursively + * + * @pr: A reference to the root powercap zones descriptors + * @spz: A reference to the SCMI powercap zone to register + * + * When registering SCMI powercap zones with the powercap framework we should + * take care to always register zones starting from the root ones and to + * deregister starting from the leaves. + * + * Unfortunately we cannot assume that the array of available SCMI powercap + * zones provided by the SCMI platform firmware is built to comply with such + * requirement. + * + * This function, given an SCMI powercap zone to register, takes care to walk + * the SCMI powercap zones tree up to the root looking recursively for + * unregistered parent zones before registering the provided zone; at the same + * time each registered zone height in such a tree is accounted for and each + * zone, once registered, is stored in the @registered_zones array that is + * indexed by zone height: this way will be trivial, at unregister time, to walk + * the @registered_zones array backward and unregister all the zones starting + * from the leaves, removing children zones before parents. + * + * While doing this, we prune away any zone marked as invalid (like the ones + * sporting an SCMI abstract power scale) as long as they are positioned as + * leaves in the SCMI powercap zones hierarchy: any non-leaf invalid zone causes + * the entire process to fail since we cannot assume the correctness of an SCMI + * powercap zones hierarchy if some of the internal nodes are missing. + * + * Note that the array of SCMI powercap zones as returned by the SCMI platform + * is known to be sane, i.e. zones relationships have been validated at the + * protocol layer. + * + * Return: 0 on Success + */ +static int scmi_powercap_register_zone(struct scmi_powercap_root *pr, + struct scmi_powercap_zone *spz) +{ + int ret = 0; + struct scmi_powercap_zone *parent; + + if (!spz->info) + return ret; + + parent = scmi_powercap_get_parent_zone(spz); + if (parent && !scmi_powercap_is_zone_registered(parent)) { + /* + * Bail out if a parent domain was marked as unsupported: + * only domains participating as leaves can be skipped. + */ + if (!parent->info) + return -ENODEV; + + ret = scmi_powercap_register_zone(pr, parent); + if (ret) + return ret; + } + + if (!scmi_powercap_is_zone_registered(spz)) { + struct powercap_zone *z; + + z = powercap_register_zone(&spz->zone, + scmi_top_pcntrl, + spz->info->name, + parent ? &parent->zone : NULL, + &zone_ops, 1, &constraint_ops); + if (!IS_ERR(z)) { + spz->height = scmi_powercap_get_zone_height(spz); + list_add(&spz->node, + &pr->registered_zones[spz->height]); + dev_dbg(spz->dev, + "Registered node %s - parent %s - height:%d\n", + spz->info->name, + parent ? parent->info->name : "ROOT", + spz->height); + ret = 0; + } else { + ret = PTR_ERR(z); + dev_err(spz->dev, + "Error registering node:%s - parent:%s - h:%d - ret:%d\n", + spz->info->name, + parent ? parent->info->name : "ROOT", + spz->height, ret); + } + } + + return ret; +} + +static int scmi_powercap_probe(struct scmi_device *sdev) +{ + int ret, i; + struct scmi_powercap_root *pr; + struct scmi_powercap_zone *spz; + struct scmi_protocol_handle *ph; + struct device *dev = &sdev->dev; + + if (!sdev->handle) + return -ENODEV; + + powercap_ops = sdev->handle->devm_protocol_get(sdev, + SCMI_PROTOCOL_POWERCAP, + &ph); + if (IS_ERR(powercap_ops)) + return PTR_ERR(powercap_ops); + + pr = devm_kzalloc(dev, sizeof(*pr), GFP_KERNEL); + if (!pr) + return -ENOMEM; + + ret = powercap_ops->num_domains_get(ph); + if (ret < 0) { + dev_err(dev, "number of powercap domains not found\n"); + return ret; + } + pr->num_zones = ret; + + pr->spzones = devm_kcalloc(dev, pr->num_zones, + sizeof(*pr->spzones), GFP_KERNEL); + if (!pr->spzones) + return -ENOMEM; + + /* Allocate for worst possible scenario of maximum tree height. */ + pr->registered_zones = devm_kcalloc(dev, pr->num_zones, + sizeof(*pr->registered_zones), + GFP_KERNEL); + if (!pr->registered_zones) + return -ENOMEM; + + for (i = 0, spz = pr->spzones; i < pr->num_zones; i++, spz++) { + /* + * Powercap domains are validate by the protocol layer, i.e. + * when only non-NULL domains are returned here, whose + * parent_id is assured to point to another valid domain. + */ + spz->info = powercap_ops->info_get(ph, i); + + spz->dev = dev; + spz->ph = ph; + spz->spzones = pr->spzones; + INIT_LIST_HEAD(&spz->node); + INIT_LIST_HEAD(&pr->registered_zones[i]); + + /* + * Forcibly skip powercap domains using an abstract scale. + * Note that only leaves domains can be skipped, so this could + * lead later to a global failure. + */ + if (!spz->info->powercap_scale_uw && + !spz->info->powercap_scale_mw) { + dev_warn(dev, + "Abstract power scale not supported. Skip %s.\n", + spz->info->name); + spz->info = NULL; + continue; + } + } + + /* + * Scan array of retrieved SCMI powercap domains and register them + * recursively starting from the root domains. + */ + for (i = 0, spz = pr->spzones; i < pr->num_zones; i++, spz++) { + ret = scmi_powercap_register_zone(pr, spz); + if (ret) { + dev_err(dev, + "Failed to register powercap zone %s - ret:%d\n", + spz->info->name, ret); + scmi_powercap_unregister_all_zones(pr); + return ret; + } + } + + dev_set_drvdata(dev, pr); + + dev_info(dev, "Registered %d SCMI Powercap domains !\n", pr->num_zones); + + return ret; +} + +static void scmi_powercap_remove(struct scmi_device *sdev) +{ + struct device *dev = &sdev->dev; + struct scmi_powercap_root *pr = dev_get_drvdata(dev); + + scmi_powercap_unregister_all_zones(pr); +} + +static const struct scmi_device_id scmi_id_table[] = { + { SCMI_PROTOCOL_POWERCAP, "powercap" }, + { }, +}; +MODULE_DEVICE_TABLE(scmi, scmi_id_table); + +static struct scmi_driver scmi_powercap_driver = { + .name = "scmi-powercap", + .probe = scmi_powercap_probe, + .remove = scmi_powercap_remove, + .id_table = scmi_id_table, +}; + +static int __init scmi_powercap_init(void) +{ + int ret; + + scmi_top_pcntrl = powercap_register_control_type(NULL, "arm-scmi", NULL); + if (IS_ERR(scmi_top_pcntrl)) + return PTR_ERR(scmi_top_pcntrl); + + ret = scmi_register(&scmi_powercap_driver); + if (ret) + powercap_unregister_control_type(scmi_top_pcntrl); + + return ret; +} +module_init(scmi_powercap_init); + +static void __exit scmi_powercap_exit(void) +{ + scmi_unregister(&scmi_powercap_driver); + + powercap_unregister_control_type(scmi_top_pcntrl); +} +module_exit(scmi_powercap_exit); + +MODULE_AUTHOR("Cristian Marussi "); +MODULE_DESCRIPTION("ARM SCMI Powercap driver"); +MODULE_LICENSE("GPL"); From 7ac70300f386a5f7d00f66ccc46ed30576b68ef4 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 1 Nov 2022 22:14:02 +0100 Subject: [PATCH 02/11] powercap: Use kstrtobool() instead of strtobool() strtobool() is the same as kstrtobool(). However, the latter is more used within the kernel. In order to remove strtobool() and slightly simplify kstrtox.h, switch to the other function name. While at it, include the corresponding header file () Signed-off-by: Christophe JAILLET Signed-off-by: Rafael J. Wysocki --- drivers/powercap/powercap_sys.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/powercap/powercap_sys.c b/drivers/powercap/powercap_sys.c index f0654a932b37..1f968353d479 100644 --- a/drivers/powercap/powercap_sys.c +++ b/drivers/powercap/powercap_sys.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -446,7 +447,7 @@ static ssize_t enabled_store(struct device *dev, { bool mode; - if (strtobool(buf, &mode)) + if (kstrtobool(buf, &mode)) return -EINVAL; if (dev->parent) { struct powercap_zone *power_zone = to_powercap_zone(dev); From d0c46b59ac82cb2c81c91188cb100401c1862d53 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 2 Nov 2022 16:19:33 +0530 Subject: [PATCH 03/11] dt-bindings: opp: Fix usage of current in microwatt property The bindings mentions "current" instead of "power". Fix it. Tested-by: James Calligeros Signed-off-by: Viresh Kumar --- Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml index 66d0ec763f0b..cb025b0a346d 100644 --- a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml +++ b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml @@ -108,7 +108,7 @@ patternProperties: The power for the OPP in micro-Watts. Entries for multiple regulators shall be provided in the same field - separated by angular brackets <>. If current values aren't required + separated by angular brackets <>. If power values aren't required for a regulator, then it shall be filled with 0. If power values aren't required for any of the regulators, then this field is not required. The OPP binding doesn't provide any provisions to relate the From 1b91dba2474ed61c34e453a48c4968833ebd68c6 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 2 Nov 2022 16:19:33 +0530 Subject: [PATCH 04/11] dt-bindings: opp: Fix named microwatt property The named microwatt- property should look exactly like microvolt and microamp properties. There were some differences, fix them. Tested-by: James Calligeros Signed-off-by: Viresh Kumar --- Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml index cb025b0a346d..cf9c2f7bddc2 100644 --- a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml +++ b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml @@ -230,9 +230,9 @@ patternProperties: minItems: 1 maxItems: 8 # Should be enough regulators - '^opp-microwatt': + '^opp-microwatt-': description: - Named opp-microwatt property. Similar to opp-microamp property, + Named opp-microwatt property. Similar to opp-microamp- property, but for microwatt instead. $ref: /schemas/types.yaml#/definitions/uint32-array minItems: 1 From 71b09429e82e5cd402bedc763c25b5962fdc9d71 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 2 Nov 2022 16:57:05 +0530 Subject: [PATCH 05/11] OPP: Parse named opp-microwatt property too We missed parsing the named opp-microwatt- property, fix that. Tested-by: James Calligeros Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 605d68673f92..e010e119c42b 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -695,9 +695,19 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, } } - /* Search for "opp-microwatt" */ - sprintf(name, "opp-microwatt"); - prop = of_find_property(opp->np, name, NULL); + /* Search for "opp-microwatt-" */ + prop = NULL; + if (opp_table->prop_name) { + snprintf(name, sizeof(name), "opp-microwatt-%s", + opp_table->prop_name); + prop = of_find_property(opp->np, name, NULL); + } + + if (!prop) { + /* Search for "opp-microwatt" */ + sprintf(name, "opp-microwatt"); + prop = of_find_property(opp->np, name, NULL); + } if (prop) { pcount = of_property_count_u32_elems(opp->np, name); From e5acb1991b5884d86f113d7841024234c465c501 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 2 Nov 2022 16:47:08 +0530 Subject: [PATCH 06/11] OPP: Simplify opp_parse_supplies() by restructuring it opp_parse_supplies() has grown into too big of a routine (~190 lines) and it is not straight-forward to understand it anymore. Break it into smaller routines and reduce code redundancy a bit by using the same code to parse properties. This shouldn't result in any logical changes. Tested-by: James Calligeros Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 266 +++++++++++++++++++---------------------------- 1 file changed, 106 insertions(+), 160 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index e010e119c42b..08d97b97c326 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -578,179 +578,126 @@ static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table, return false; } +static u32 *_parse_named_prop(struct dev_pm_opp *opp, struct device *dev, + struct opp_table *opp_table, + const char *prop_type, bool *triplet) +{ + struct property *prop = NULL; + char name[NAME_MAX]; + int count, ret; + u32 *out; + + /* Search for "opp--" */ + if (opp_table->prop_name) { + snprintf(name, sizeof(name), "opp-%s-%s", prop_type, + opp_table->prop_name); + prop = of_find_property(opp->np, name, NULL); + } + + if (!prop) { + /* Search for "opp-" */ + snprintf(name, sizeof(name), "opp-%s", prop_type); + prop = of_find_property(opp->np, name, NULL); + if (!prop) + return NULL; + } + + count = of_property_count_u32_elems(opp->np, name); + if (count < 0) { + dev_err(dev, "%s: Invalid %s property (%d)\n", __func__, name, + count); + return ERR_PTR(count); + } + + /* + * Initialize regulator_count, if regulator information isn't provided + * by the platform. Now that one of the properties is available, fix the + * regulator_count to 1. + */ + if (unlikely(opp_table->regulator_count == -1)) + opp_table->regulator_count = 1; + + if (count != opp_table->regulator_count && + (!triplet || count != opp_table->regulator_count * 3)) { + dev_err(dev, "%s: Invalid number of elements in %s property (%u) with supplies (%d)\n", + __func__, prop_type, count, opp_table->regulator_count); + return ERR_PTR(-EINVAL); + } + + out = kmalloc_array(count, sizeof(*out), GFP_KERNEL); + if (!out) + return ERR_PTR(-EINVAL); + + ret = of_property_read_u32_array(opp->np, name, out, count); + if (ret) { + dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret); + kfree(out); + return ERR_PTR(-EINVAL); + } + + if (triplet) + *triplet = count != opp_table->regulator_count; + + return out; +} + +static u32 *opp_parse_microvolt(struct dev_pm_opp *opp, struct device *dev, + struct opp_table *opp_table, bool *triplet) +{ + u32 *microvolt; + + microvolt = _parse_named_prop(opp, dev, opp_table, "microvolt", triplet); + if (IS_ERR(microvolt)) + return microvolt; + + if (!microvolt) { + /* + * Missing property isn't a problem, but an invalid + * entry is. This property isn't optional if regulator + * information is provided. + */ + if (opp_table->regulator_count > 0) { + dev_err(dev, "%s: opp-microvolt missing although OPP managing regulators\n", + __func__); + return ERR_PTR(-EINVAL); + } + } + + return microvolt; +} + static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, struct opp_table *opp_table) { - u32 *microvolt, *microamp = NULL, *microwatt = NULL; - int supplies = opp_table->regulator_count; - int vcount, icount, pcount, ret, i, j; - struct property *prop = NULL; - char name[NAME_MAX]; + u32 *microvolt, *microamp, *microwatt; + int ret = 0, i, j; + bool triplet; - /* Search for "opp-microvolt-" */ - if (opp_table->prop_name) { - snprintf(name, sizeof(name), "opp-microvolt-%s", - opp_table->prop_name); - prop = of_find_property(opp->np, name, NULL); - } + microvolt = opp_parse_microvolt(opp, dev, opp_table, &triplet); + if (IS_ERR_OR_NULL(microvolt)) + return PTR_ERR(microvolt); - if (!prop) { - /* Search for "opp-microvolt" */ - sprintf(name, "opp-microvolt"); - prop = of_find_property(opp->np, name, NULL); - - /* Missing property isn't a problem, but an invalid entry is */ - if (!prop) { - if (unlikely(supplies == -1)) { - /* Initialize regulator_count */ - opp_table->regulator_count = 0; - return 0; - } - - if (!supplies) - return 0; - - dev_err(dev, "%s: opp-microvolt missing although OPP managing regulators\n", - __func__); - return -EINVAL; - } - } - - if (unlikely(supplies == -1)) { - /* Initialize regulator_count */ - supplies = opp_table->regulator_count = 1; - } else if (unlikely(!supplies)) { - dev_err(dev, "%s: opp-microvolt wasn't expected\n", __func__); - return -EINVAL; - } - - vcount = of_property_count_u32_elems(opp->np, name); - if (vcount < 0) { - dev_err(dev, "%s: Invalid %s property (%d)\n", - __func__, name, vcount); - return vcount; - } - - /* There can be one or three elements per supply */ - if (vcount != supplies && vcount != supplies * 3) { - dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n", - __func__, name, vcount, supplies); - return -EINVAL; - } - - microvolt = kmalloc_array(vcount, sizeof(*microvolt), GFP_KERNEL); - if (!microvolt) - return -ENOMEM; - - ret = of_property_read_u32_array(opp->np, name, microvolt, vcount); - if (ret) { - dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret); - ret = -EINVAL; + microamp = _parse_named_prop(opp, dev, opp_table, "microamp", NULL); + if (IS_ERR(microamp)) { + ret = PTR_ERR(microamp); goto free_microvolt; } - /* Search for "opp-microamp-" */ - prop = NULL; - if (opp_table->prop_name) { - snprintf(name, sizeof(name), "opp-microamp-%s", - opp_table->prop_name); - prop = of_find_property(opp->np, name, NULL); + microwatt = _parse_named_prop(opp, dev, opp_table, "microwatt", NULL); + if (IS_ERR(microwatt)) { + ret = PTR_ERR(microwatt); + goto free_microamp; } - if (!prop) { - /* Search for "opp-microamp" */ - sprintf(name, "opp-microamp"); - prop = of_find_property(opp->np, name, NULL); - } - - if (prop) { - icount = of_property_count_u32_elems(opp->np, name); - if (icount < 0) { - dev_err(dev, "%s: Invalid %s property (%d)\n", __func__, - name, icount); - ret = icount; - goto free_microvolt; - } - - if (icount != supplies) { - dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n", - __func__, name, icount, supplies); - ret = -EINVAL; - goto free_microvolt; - } - - microamp = kmalloc_array(icount, sizeof(*microamp), GFP_KERNEL); - if (!microamp) { - ret = -EINVAL; - goto free_microvolt; - } - - ret = of_property_read_u32_array(opp->np, name, microamp, - icount); - if (ret) { - dev_err(dev, "%s: error parsing %s: %d\n", __func__, - name, ret); - ret = -EINVAL; - goto free_microamp; - } - } - - /* Search for "opp-microwatt-" */ - prop = NULL; - if (opp_table->prop_name) { - snprintf(name, sizeof(name), "opp-microwatt-%s", - opp_table->prop_name); - prop = of_find_property(opp->np, name, NULL); - } - - if (!prop) { - /* Search for "opp-microwatt" */ - sprintf(name, "opp-microwatt"); - prop = of_find_property(opp->np, name, NULL); - } - - if (prop) { - pcount = of_property_count_u32_elems(opp->np, name); - if (pcount < 0) { - dev_err(dev, "%s: Invalid %s property (%d)\n", __func__, - name, pcount); - ret = pcount; - goto free_microamp; - } - - if (pcount != supplies) { - dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n", - __func__, name, pcount, supplies); - ret = -EINVAL; - goto free_microamp; - } - - microwatt = kmalloc_array(pcount, sizeof(*microwatt), - GFP_KERNEL); - if (!microwatt) { - ret = -EINVAL; - goto free_microamp; - } - - ret = of_property_read_u32_array(opp->np, name, microwatt, - pcount); - if (ret) { - dev_err(dev, "%s: error parsing %s: %d\n", __func__, - name, ret); - ret = -EINVAL; - goto free_microwatt; - } - } - - for (i = 0, j = 0; i < supplies; i++) { + for (i = 0, j = 0; i < opp_table->regulator_count; i++) { opp->supplies[i].u_volt = microvolt[j++]; - if (vcount == supplies) { - opp->supplies[i].u_volt_min = opp->supplies[i].u_volt; - opp->supplies[i].u_volt_max = opp->supplies[i].u_volt; - } else { + if (triplet) { opp->supplies[i].u_volt_min = microvolt[j++]; opp->supplies[i].u_volt_max = microvolt[j++]; + } else { + opp->supplies[i].u_volt_min = opp->supplies[i].u_volt; + opp->supplies[i].u_volt_max = opp->supplies[i].u_volt; } if (microamp) @@ -760,7 +707,6 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, opp->supplies[i].u_watt = microwatt[i]; } -free_microwatt: kfree(microwatt); free_microamp: kfree(microamp); From 2eedf62e66c28f6991425b580af68b3d89fa8021 Mon Sep 17 00:00:00 2001 From: James Calligeros Date: Thu, 3 Nov 2022 14:40:44 +0530 Subject: [PATCH 07/11] OPP: decouple dt properties in opp_parse_supplies() The opp-microwatt property was added with the intention of providing platforms a way to specify a precise value for the power consumption of a device at a given OPP to enable better energy-aware scheduling decisions by informing the kernel of the total static and dynamic power of a device at a given OPP, removing the reliance on the EM subsystem's often flawed estimations. This property is parsed by opp_parse_supplies(), which creates a hard dependency on the opp-microvolt property. Some platforms, such as Apple Silicon, do not describe their device's voltage regulators in the DT as they cannot be controlled by the kernel and/or rely on opaque firmware algorithms to control their voltage and current characteristics at runtime. We can, however, experimentally determine the power consumption of a given device at a given OPP, taking advantage of opp-microwatt to provide EAS on such devices as was initially intended. Allow platforms to specify and consume any subset of opp-microvolt, opp-microamp, or opp-microwatt without a hard dependency on opp-microvolt to enable this functionality on such platforms. Tested-by: James Calligeros Signed-off-by: James Calligeros Co-developed-by: Viresh Kumar Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 08d97b97c326..e55c6095adf0 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -654,9 +654,12 @@ static u32 *opp_parse_microvolt(struct dev_pm_opp *opp, struct device *dev, /* * Missing property isn't a problem, but an invalid * entry is. This property isn't optional if regulator - * information is provided. + * information is provided. Check only for the first OPP, as + * regulator_count may get initialized after that to a valid + * value. */ - if (opp_table->regulator_count > 0) { + if (list_empty(&opp_table->opp_list) && + opp_table->regulator_count > 0) { dev_err(dev, "%s: opp-microvolt missing although OPP managing regulators\n", __func__); return ERR_PTR(-EINVAL); @@ -674,7 +677,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, bool triplet; microvolt = opp_parse_microvolt(opp, dev, opp_table, &triplet); - if (IS_ERR_OR_NULL(microvolt)) + if (IS_ERR(microvolt)) return PTR_ERR(microvolt); microamp = _parse_named_prop(opp, dev, opp_table, "microamp", NULL); @@ -689,15 +692,26 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, goto free_microamp; } - for (i = 0, j = 0; i < opp_table->regulator_count; i++) { - opp->supplies[i].u_volt = microvolt[j++]; + /* + * Initialize regulator_count if it is uninitialized and no properties + * are found. + */ + if (unlikely(opp_table->regulator_count == -1)) { + opp_table->regulator_count = 0; + return 0; + } - if (triplet) { - opp->supplies[i].u_volt_min = microvolt[j++]; - opp->supplies[i].u_volt_max = microvolt[j++]; - } else { - opp->supplies[i].u_volt_min = opp->supplies[i].u_volt; - opp->supplies[i].u_volt_max = opp->supplies[i].u_volt; + for (i = 0, j = 0; i < opp_table->regulator_count; i++) { + if (microvolt) { + opp->supplies[i].u_volt = microvolt[j++]; + + if (triplet) { + opp->supplies[i].u_volt_min = microvolt[j++]; + opp->supplies[i].u_volt_max = microvolt[j++]; + } else { + opp->supplies[i].u_volt_min = opp->supplies[i].u_volt; + opp->supplies[i].u_volt_max = opp->supplies[i].u_volt; + } } if (microamp) From dba79b78ecc18f7788fd08eb998388e226817fb5 Mon Sep 17 00:00:00 2001 From: Serge Semin Date: Mon, 7 Nov 2022 23:43:54 +0300 Subject: [PATCH 08/11] dt-bindings: opp-v2: Fix clock-latency-ns prop in example Accidentally discovered a hidden typo in the DT-nodes defined in the opp-v2 bindings example. Instead of specifying the "clock-latency-ns" property the DT-node has been created with the "lock-latency-ns" property in it, which doesn't exist neither in opp-v2 nor in the base schemas. Let's fix the name to having the "clock-" prefix as it was originally implied and as the rest of the similar nodes has. Fixes: 94274f20f6bf ("dt-bindings: opp: Convert to DT schema") Signed-off-by: Serge Semin Signed-off-by: Viresh Kumar --- Documentation/devicetree/bindings/opp/opp-v2.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/opp/opp-v2.yaml b/Documentation/devicetree/bindings/opp/opp-v2.yaml index eaf8fba2c691..2f05920fce79 100644 --- a/Documentation/devicetree/bindings/opp/opp-v2.yaml +++ b/Documentation/devicetree/bindings/opp/opp-v2.yaml @@ -155,7 +155,7 @@ examples: opp-hz = /bits/ 64 <1200000000>; opp-microvolt = <1025000>; opp-microamp = <90000>; - lock-latency-ns = <290000>; + clock-latency-ns = <290000>; turbo-mode; }; }; From 9c252ecf30360cb7b4dbcc275aebe5642174fd39 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Tue, 22 Nov 2022 09:00:14 +0200 Subject: [PATCH 09/11] platform/x86: intel-uncore-freq: add Emerald Rapids support Make Intel uncore frequency driver support Emerald Rapids by adding its CPU model to the match table. Emerald Rapids uncore frequency control is the same as in Sapphire Rapids. Signed-off-by: Artem Bityutskiy Acked-by: Srinivas Pandruvada Acked-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c index 8f9c571d7257..00ac7e381441 100644 --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c @@ -203,6 +203,7 @@ static const struct x86_cpu_id intel_uncore_cpu_ids[] = { X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X, NULL), X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D, NULL), X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, NULL), + X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X, NULL), {} }; MODULE_DEVICE_TABLE(x86cpu, intel_uncore_cpu_ids); From f9b4dc920d35cca8bf85ecce836ee6d23788e7cf Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Mon, 7 Nov 2022 11:56:57 +0100 Subject: [PATCH 10/11] notifier: repair slips in kernel-doc comments Invoking ./scripts/kernel-doc -none kernel/notifier.c warns: kernel/notifier.c:71: warning: Excess function parameter 'returns' description in 'notifier_call_chain' kernel/notifier.c:119: warning: Function parameter or member 'v' not described in 'notifier_call_chain_robust' These two warning are easy to fix, as they are just due to some minor slips that makes the comment not follow kernel-doc's syntactic expectation. Fix those minor slips in kernel-doc comments for make W=1 happiness. Signed-off-by: Lukas Bulwahn Signed-off-by: Rafael J. Wysocki --- kernel/notifier.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/notifier.c b/kernel/notifier.c index 0d5bd62c480e..ab75637fd904 100644 --- a/kernel/notifier.c +++ b/kernel/notifier.c @@ -62,7 +62,7 @@ static int notifier_chain_unregister(struct notifier_block **nl, * value of this parameter is -1. * @nr_calls: Records the number of notifications sent. Don't care * value of this field is NULL. - * @returns: notifier_call_chain returns the value returned by the + * Return: notifier_call_chain returns the value returned by the * last notifier function called. */ static int notifier_call_chain(struct notifier_block **nl, @@ -105,13 +105,13 @@ NOKPROBE_SYMBOL(notifier_call_chain); * @val_up: Value passed unmodified to the notifier function * @val_down: Value passed unmodified to the notifier function when recovering * from an error on @val_up - * @v Pointer passed unmodified to the notifier function + * @v: Pointer passed unmodified to the notifier function * * NOTE: It is important the @nl chain doesn't change between the two * invocations of notifier_call_chain() such that we visit the * exact same notifier callbacks; this rules out any RCU usage. * - * Returns: the return value of the @val_up call. + * Return: the return value of the @val_up call. */ static int notifier_call_chain_robust(struct notifier_block **nl, unsigned long val_up, unsigned long val_down, From 98e596fc85fe74067bfb0a910b3d813784132a22 Mon Sep 17 00:00:00 2001 From: Srinivas Pandruvada Date: Tue, 29 Nov 2022 15:08:21 -0800 Subject: [PATCH 11/11] powercap: idle_inject: Fix warnings with make W=1 Fix following warning at three places: Function parameter or member 'ii_dev' not described. Signed-off-by: Srinivas Pandruvada Signed-off-by: Rafael J. Wysocki --- drivers/powercap/idle_inject.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/powercap/idle_inject.c b/drivers/powercap/idle_inject.c index 999e218d7793..fe86a09e3b67 100644 --- a/drivers/powercap/idle_inject.c +++ b/drivers/powercap/idle_inject.c @@ -147,6 +147,7 @@ static void idle_inject_fn(unsigned int cpu) /** * idle_inject_set_duration - idle and run duration update helper + * @ii_dev: idle injection control device structure * @run_duration_us: CPU run time to allow in microseconds * @idle_duration_us: CPU idle time to inject in microseconds */ @@ -162,6 +163,7 @@ void idle_inject_set_duration(struct idle_inject_device *ii_dev, /** * idle_inject_get_duration - idle and run duration retrieval helper + * @ii_dev: idle injection control device structure * @run_duration_us: memory location to store the current CPU run time * @idle_duration_us: memory location to store the current CPU idle time */ @@ -175,6 +177,7 @@ void idle_inject_get_duration(struct idle_inject_device *ii_dev, /** * idle_inject_set_latency - set the maximum latency allowed + * @ii_dev: idle injection control device structure * @latency_us: set the latency requirement for the idle state */ void idle_inject_set_latency(struct idle_inject_device *ii_dev,