Re: [PATCH v2 3/4] PM / devfreq: Fix governor_store() failing when device has no current governor

[Jie Zhan]

On 4/1/2026 11:31 AM, Yaxiong Tian wrote:
> Since devfreq_remove_governor() may clear the device's current governor
> in certain situations, while governors actually exist independently
> of the device, directly returning EINVAL in this case is inaccurate.
> 
> To fix this issue, remove this check and add relevant logic for when
> df->governor is NULL.
> 
> Fixes: 483d557ee9a3 ("PM / devfreq: Clean up the devfreq instance name in sysfs attr")
> Signed-off-by: Yaxiong Tian <tianyaxiong@xxxxxxxxxx>
> ---
>  drivers/devfreq/devfreq.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 2977b07be939..975f82d7a9d1 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1390,9 +1390,6 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>  	char str_governor[DEVFREQ_NAME_LEN + 1];
>  	const struct devfreq_governor *governor, *prev_governor;
>  
> -	if (!df->governor)
> -		return -EINVAL;
> -
>  	ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor);
>  	if (ret != 1)
>  		return -EINVAL;
> @@ -1403,6 +1400,20 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>  		ret = PTR_ERR(governor);
>  		goto out;
>  	}
> +
> +	if (!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);
> +			df->governor = NULL;
> +		} else {
> +			ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group);
> +		}
> +		goto out;
> +	}
> +
>  	if (df->governor == governor) {
>  		ret = 0;
>  		goto out;
Hi Yaxiong,

I'd suggest something like this, such that the main body of the function is
not changed.

Note that this is just an example. Not tested or carefully checked.

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 2977b07be939..aa0f1db3225c 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1390,9 +1390,6 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
        char str_governor[DEVFREQ_NAME_LEN + 1];
        const struct devfreq_governor *governor, *prev_governor;

-       if (!df->governor)
-               return -EINVAL;
-
        ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor);
        if (ret != 1)
                return -EINVAL;
@@ -1403,6 +1400,10 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
                ret = PTR_ERR(governor);
                goto out;
        }
+
+       if (!df->governor)
+               goto skip_stop;
+
        if (df->governor == governor) {
                ret = 0;
                goto out;
@@ -1423,6 +1424,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
                goto out;
        }

+skip_stop:
        /*
         * Start the new governor and create the specific sysfs files
         * which depend on the new governor.
@@ -1435,6 +1437,8 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
                         __func__, df->governor->name, ret);

                /* Restore previous governor */
+               if (!prev_governor)
+                       goto out;
                df->governor = prev_governor;
                ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
                if (ret) {