[PATCH v4 3/4] devfreq: Factor out devfreq_set_governor()
From: Jie Zhan <zhanjie9@hisilicon.com>
Date: 2026-07-29 10:31:18
Also in:
linux-pm
Subsystem:
device frequency (devfreq), the rest · Maintainers:
MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Linus Torvalds
governor_store() and devfreq_add_device() contain similar logic for setting a governor when devfreq->governor is NULL. Merge this into a common function, devfreq_set_governor(), to reduce code duplication and unify the entry of setting governors. This also prepares for further changes that get / put a module refcount of the active governor and prevent the governor module from being unloaded while it's in use. Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com> --- drivers/devfreq/devfreq.c | 128 ++++++++++++++++++++------------------ 1 file changed, 67 insertions(+), 61 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 53c40d795a13..959f0b08ed36 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c@@ -318,6 +318,70 @@ static struct devfreq_governor *try_then_request_governor(const char *name) return governor; } +static int devfreq_set_governor(struct devfreq *df, + const struct devfreq_governor *new_gov) +{ + const struct devfreq_governor *old_gov; + struct device *dev; + int ret; + + lockdep_assert_held(&devfreq_list_lock); + + old_gov = df->governor; + dev = &df->dev; + + if (old_gov) { + if (old_gov == new_gov) + return 0; + + if (IS_SUPPORTED_FLAG(old_gov->flags, IMMUTABLE) || + IS_SUPPORTED_FLAG(new_gov->flags, IMMUTABLE)) + return -EINVAL; + + /* Stop the current governor */ + ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL); + if (ret) { + dev_warn(dev, "%s: Governor %s not stopped(%d)\n", + __func__, df->governor->name, ret); + return ret; + } + } + + /* Start the new governor */ + df->governor = new_gov; + ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL); + if (ret) { + dev_warn(dev, "%s: Governor %s not started(%d)\n", + __func__, df->governor->name, ret); + + /* Restore previous governor */ + df->governor = old_gov; + if (!df->governor) + return ret; + + ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL); + if (ret) { + dev_err(dev, "%s: restore Governor %s failed (%d)\n", + __func__, df->governor->name, ret); + df->governor = NULL; + return ret; + } + + ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group); + if (ret) + dev_warn(dev, "sysfs update failed (%d)\n", ret); + + /* + * Old governor restored successfully, but the new governor + * failed to start. Return a distinct error so the caller is + * not misled into thinking the switch succeeded. + */ + return -EAGAIN; + } + + return sysfs_update_group(&df->dev.kobj, &gov_attr_group); +} + static int devfreq_notify_transition(struct devfreq *devfreq, struct devfreq_freqs *freqs, unsigned int state) {
@@ -942,9 +1006,7 @@ struct devfreq *devfreq_add_device(struct device *dev, goto err_init; } - devfreq->governor = governor; - err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START, - NULL); + err = devfreq_set_governor(devfreq, governor); if (err) { dev_err_probe(dev, err, "%s: Unable to start governor for the device\n",
@@ -952,10 +1014,6 @@ struct devfreq *devfreq_add_device(struct device *dev, goto err_init; } - err = sysfs_update_group(&devfreq->dev.kobj, &gov_attr_group); - if (err) - goto err_init; - list_add(&devfreq->node, &devfreq_list); mutex_unlock(&devfreq_list_lock);
@@ -1380,7 +1438,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, struct devfreq *df = to_devfreq(dev); int ret; char str_governor[DEVFREQ_NAME_LEN + 1]; - const struct devfreq_governor *governor, *prev_governor; + const struct devfreq_governor *governor; ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor); if (ret != 1)
@@ -1391,59 +1449,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, if (IS_ERR(governor)) return PTR_ERR(governor); - if (!df->governor) - goto start_new_governor; - - if (df->governor == governor) - return count; - - if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE) || - IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE)) - return -EINVAL; - - /* - * Stop the current governor and remove the specific sysfs files - * which depend on current governor. - */ - ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL); - if (ret) { - dev_warn(dev, "%s: Governor %s not stopped(%d)\n", - __func__, df->governor->name, ret); - return ret; - } - -start_new_governor: - /* - * Start the new governor and create the specific sysfs files - * which depend on the new governor. - */ - prev_governor = df->governor; - df->governor = governor; - ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL); - if (ret) { - dev_warn(dev, "%s: Governor %s not started(%d)\n", - __func__, df->governor->name, ret); - - /* Restore previous governor */ - df->governor = prev_governor; - if (!df->governor) - return ret; - - ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL); - if (ret) { - dev_err(dev, - "%s: reverting to Governor %s failed (%d)\n", - __func__, prev_governor->name, ret); - df->governor = NULL; - return ret; - } - } - - /* - * Create the sysfs files for the new governor. But if failed to start - * the new governor, restore the sysfs files of previous governor. - */ - ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group); + ret = devfreq_set_governor(df, governor); if (ret) return ret;
--
2.43.0