Hello Chanwoo,
20.10.2020 06:04, Chanwoo Choi пишет:
> @@ -1354,55 +1365,71 @@ 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 *new_governor, *prev_governor;
>
> if (!df->governor)
> return -EINVAL;
> + prev_governor = df->governor;
>
> ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor);
> if (ret != 1)
> return -EINVAL;
>
> mutex_lock(&devfreq_list_lock);
> - governor = try_then_request_governor(str_governor);
> - if (IS_ERR(governor)) {
> - ret = PTR_ERR(governor);
> + new_governor = try_then_request_governor(str_governor);
> + if (IS_ERR(new_governor)) {
> + ret = PTR_ERR(new_governor);
> goto out;
> }
> - if (df->governor == governor) {
> + if (prev_governor == new_governor) {
> ret = 0;
> goto out;
> - } else if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)
> - || IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE)) {
> + } else if (IS_SUPPORTED_FLAG(prev_governor->flags, IMMUTABLE)
> + || IS_SUPPORTED_FLAG(new_governor->flags, IMMUTABLE)) {
> ret = -EINVAL;
> goto out;
> }
>
> - ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
> + /*
> + * Stop the previous governor and remove the specific sysfs files
> + * which depend on previous governor.
> + */
> + ret = prev_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);
> + __func__, prev_governor->name, ret);
> goto out;
> }
>
> - prev_governor = df->governor;
> - df->governor = governor;
> - strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN);
> - ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
> + remove_sysfs_files(df, prev_governor);
> +
> + /*
> + * Start the new governor and create the specific sysfs files
> + * which depend on new governor.
> + */
> + strncpy(df->governor_name, new_governor->name, DEVFREQ_NAME_LEN);
> + ret = new_governor->event_handler(df, DEVFREQ_GOV_START, NULL);
The "df->governor = new_governor" needs to be set before invocation of
the event_handler(GOV_START) because governors like a
performance-governor need to have the df->governor being set before the
GOV_START callback, otherwise performance governor will use
devfreq->prev_governor->get_target_freq() in devfreq_update_target(),
thus setting a wrong (non-max) freq.
> if (ret) {
> dev_warn(dev, "%s: Governor %s not started(%d)\n",
> - __func__, df->governor->name, ret);
> - df->governor = prev_governor;
> + __func__, new_governor->name, ret);
> strncpy(df->governor_name, prev_governor->name,
> DEVFREQ_NAME_LEN);
> - ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
Same here.
