[PATCH v2 0/4] Fix some errors in the devfreq core layer when governor is NULL

[PATCH v2 0/4] Fix some errors in the devfreq core layer when governor is NULL

[Yaxiong Tian]

While doing some development work with devfreq_add_governor()/
devfreq_remove_governor(), I discovered several bugs caused when
devfreq->governor is NULL. Specifically:

1) A possible null pointer issue in devfreq_add_governor(), caused
by devfreq_remove_governor() setting devfreq->governor to NULL in
certain situations, while devfreq_add_governor() lacks corresponding
checks for devfreq->governor.

2) When operating on governor and available_governors under /sys,
there are also some unexpected errors.

For issue 1, the solution is similar to Jie Zhan's PATCH.
https://lore.kernel.org/linux-pm/20260326123428.800407-6-zhanjie9@hisilicon.com/

The original v1 approach actually changed the original logic, which is incorrect.

See the following patches for details.

change in v2:
1) rebase to  linux-next(20260326) /devfreq-next 
2) remove some code to fix null pointer in patch1
3) add  sysfs_update_group() in patch2


Yaxiong Tian (4):
  PM / devfreq: Fix possible null pointer issue in
    devfreq_add_governor()
  PM / devfreq: Fix available_governors_show() when no governor is set
  PM / devfreq: Fix governor_store() failing when device has no current
    governor
  PM / devfreq: Optimize error return value of governor_show()

 drivers/devfreq/devfreq.c | 57 +++++++++++----------------------------
 1 file changed, 16 insertions(+), 41 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] PM / devfreq: Fix possible null pointer issue in devfreq_add_governor()

[Yaxiong Tian]

When a user removes a governor using devfreq_remove_governor(), if
the current device is using this governor, devfreq->governor will
be set to NULL. When the user registers any governor
using devfreq_add_governor(), since devfreq->governor is NULL, a
null pointer error occurs in strncmp().

For example: A user loads the userspace gov through a module, then
a device selects userspace. When unloading the userspace module and
then loading it again, the null pointer error occurs:

Unable to handle kernel NULL pointer dereference at virtual address
0000000000000010
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
*******************skip *********************
Call trace:
__pi_strncmp+0x20/0x1b8
devfreq_userspace_init+0x1c/0xff8 [governor_userspace]
do_one_initcall+0x4c/0x278
do_init_module+0x5c/0x218
load_module+0x1f1c/0x1fc8
init_module_from_file+0x8c/0xd0
__arm64_sys_finit_module+0x220/0x3d8
invoke_syscall+0x48/0x110
el0_svc_common.constprop.0+0xbc/0xe8
do_el0_svc+0x20/0x30
el0_svc+0x24/0xb8
el0t_64_sync_handler+0xb8/0xc0
el0t_64_sync+0x14c/0x150

To fix this issue, remove the list_for_each_entry() logic, as
find_devfreq_governor() has already checked for the existence of
governor with the same name. This makes it impossible to find a
duplicate governor in the list, so the subsequent logic is
unreachable and can be removed.

Fixes: 1b5c1be2c88e ("PM / devfreq: map devfreq drivers to governor using name")
Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
 drivers/devfreq/devfreq.c | 33 ---------------------------------
 1 file changed, 33 deletions(-)

Hi Jie Zhan: 
	If you're willing, I'd like to add your Co-developed-by tag.

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 82dd9a43dc62..994984f7b6e1 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1261,7 +1261,6 @@ void devfreq_resume(void)
 int devfreq_add_governor(struct devfreq_governor *governor)
 {
 	struct devfreq_governor *g;
-	struct devfreq *devfreq;
 	int err = 0;
 
 	if (!governor) {
@@ -1280,38 +1279,6 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 
 	list_add(&governor->node, &devfreq_governor_list);
 
-	list_for_each_entry(devfreq, &devfreq_list, node) {
-		int ret = 0;
-		struct device *dev = devfreq->dev.parent;
-
-		if (!strncmp(devfreq->governor->name, governor->name,
-			     DEVFREQ_NAME_LEN)) {
-			/* The following should never occur */
-			if (devfreq->governor) {
-				dev_warn(dev,
-					 "%s: Governor %s already present\n",
-					 __func__, devfreq->governor->name);
-				ret = devfreq->governor->event_handler(devfreq,
-							DEVFREQ_GOV_STOP, NULL);
-				if (ret) {
-					dev_warn(dev,
-						 "%s: Governor %s stop = %d\n",
-						 __func__,
-						 devfreq->governor->name, ret);
-				}
-				/* Fall through */
-			}
-			devfreq->governor = governor;
-			ret = devfreq->governor->event_handler(devfreq,
-						DEVFREQ_GOV_START, NULL);
-			if (ret) {
-				dev_warn(dev, "%s: Governor %s start=%d\n",
-					 __func__, devfreq->governor->name,
-					 ret);
-			}
-		}
-	}
-
 err_out:
 	mutex_unlock(&devfreq_list_lock);
 
-- 
2.25.1

[PATCH v2 2/4] PM / devfreq: Fix available_governors_show() when no governor is set

[Yaxiong Tian]

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 use df->governor for validity
verification in the following code.

Fixes: 483d557ee9a3 ("PM / devfreq: Clean up the devfreq instance name in sysfs attr")
Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
 drivers/devfreq/devfreq.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 994984f7b6e1..2977b07be939 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1468,16 +1468,13 @@ static ssize_t available_governors_show(struct device *d,
 	struct devfreq *df = to_devfreq(d);
 	ssize_t count = 0;
 
-	if (!df->governor)
-		return -EINVAL;
-
 	mutex_lock(&devfreq_list_lock);
 
 	/*
 	 * The devfreq with immutable governor (e.g., passive) shows
 	 * only own governor.
 	 */
-	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)) {
+	if (df->governor && IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)) {
 		count = scnprintf(&buf[count], DEVFREQ_NAME_LEN,
 				  "%s ", df->governor->name);
 	/*
-- 
2.25.1

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

[Yaxiong Tian]

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@kylinos.cn>
---
 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;
-- 
2.25.1

[PATCH v2 4/4] PM / devfreq: Optimize error return value of governor_show()

[Yaxiong Tian]

When df->governor is NULL, governor_show() returns -EINVAL, which
confuses users.

To fix this issue, return -ENOENT to indicate that no governor is
currently set for the device.

Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
 drivers/devfreq/devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 975f82d7a9d1..c40568d2a4dc 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1377,7 +1377,7 @@ static ssize_t governor_show(struct device *dev,
 	struct devfreq *df = to_devfreq(dev);
 
 	if (!df->governor)
-		return -EINVAL;
+		return -ENOENT;
 
 	return sprintf(buf, "%s\n", df->governor->name);
 }
-- 
2.25.1

Re: [PATCH v2 1/4] PM / devfreq: Fix possible null pointer issue in devfreq_add_governor()

[Jie Zhan]

On 4/1/2026 11:30 AM, Yaxiong Tian wrote:
> When a user removes a governor using devfreq_remove_governor(), if
> the current device is using this governor, devfreq->governor will
> be set to NULL. When the user registers any governor
> using devfreq_add_governor(), since devfreq->governor is NULL, a
> null pointer error occurs in strncmp().
> 
> For example: A user loads the userspace gov through a module, then
> a device selects userspace. When unloading the userspace module and
> then loading it again, the null pointer error occurs:
> 
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000010
> Mem abort info:
> ESR = 0x0000000096000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> *******************skip *********************
> Call trace:
> __pi_strncmp+0x20/0x1b8
> devfreq_userspace_init+0x1c/0xff8 [governor_userspace]
> do_one_initcall+0x4c/0x278
> do_init_module+0x5c/0x218
> load_module+0x1f1c/0x1fc8
> init_module_from_file+0x8c/0xd0
> __arm64_sys_finit_module+0x220/0x3d8
> invoke_syscall+0x48/0x110
> el0_svc_common.constprop.0+0xbc/0xe8
> do_el0_svc+0x20/0x30
> el0_svc+0x24/0xb8
> el0t_64_sync_handler+0xb8/0xc0
> el0t_64_sync+0x14c/0x150
> 
> To fix this issue, remove the list_for_each_entry() logic, as
> find_devfreq_governor() has already checked for the existence of
> governor with the same name. This makes it impossible to find a
> duplicate governor in the list, so the subsequent logic is
> unreachable and can be removed.
> 
> Fixes: 1b5c1be2c88e ("PM / devfreq: map devfreq drivers to governor using name")
> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
> ---
>  drivers/devfreq/devfreq.c | 33 ---------------------------------
>  1 file changed, 33 deletions(-)
> 
> Hi Jie Zhan: 
> 	If you're willing, I'd like to add your Co-developed-by tag.
Yeah please do so.  That would be:
Co-developed-by: Jie Zhan <zhanjie9@hisilicon.com>
Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>

The rest looks good to me.

Re: [PATCH v2 2/4] PM / devfreq: Fix available_governors_show() when no governor is set

[Jie Zhan]

On 4/1/2026 11:30 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 use df->governor for validity
> verification in the following code.
> 
> Fixes: 483d557ee9a3 ("PM / devfreq: Clean up the devfreq instance name in sysfs attr")
> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
Well, you should have picked up my RB.
https://lore.kernel.org/all/baee47c8-7b29-42dd-a64a-167c19c4593b@hisilicon.com/

Some tools like b4 may make that easier.

[...]

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@kylinos.cn>
> ---
>  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) {

Re: [PATCH v2 4/4] PM / devfreq: Optimize error return value of governor_show()

[Jie Zhan]

On 4/1/2026 11:31 AM, Yaxiong Tian wrote:
> When df->governor is NULL, governor_show() returns -EINVAL, which
> confuses users.
> 
> To fix this issue, return -ENOENT to indicate that no governor is
> currently set for the device.
> 
> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
> ---
>  drivers/devfreq/devfreq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 975f82d7a9d1..c40568d2a4dc 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1377,7 +1377,7 @@ static ssize_t governor_show(struct device *dev,
>  	struct devfreq *df = to_devfreq(dev);
>  
>  	if (!df->governor)
> -		return -EINVAL;
> +		return -ENOENT;
Hi Yaxiong,

Usually we don't change sysfs error code because that might break userspace
if it depends on this to do some specific handling (although it shouldn't
do so!)

Besides that, from a user-friendliness point of view, I'd still prefer
ENODEV.

A user terminal is supposed to get "No such file" on -ENOENT and "No such
device" on -ENODEV.

I guess the latter is more meaningful since 'device' can mean 'object'?
"No such file" is a bit confusing because the sysfs file or entry is
clearly there.

Thanks,
Jie
>  
>  	return sprintf(buf, "%s\n", df->governor->name);
>  }

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

[Yaxiong Tian]

在 2026/4/2 11:21, 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@kylinos.cn>
>> ---
>>   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.
"I still prefer my modification. It's clearer and more straightforward, 
and the increase in code size is very small compared to the 
alternatives. Moreover, if we later want to implement 'cannot unload the 
last governor' (which might be the module reference approach you 
mentioned), we can simply remove this code."


>
> 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) {

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

[Jie Zhan]

On 4/2/2026 1:47 PM, Yaxiong Tian wrote:
> 
> 在 2026/4/2 11:21, 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@kylinos.cn>
>>> ---
>>>   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.
> "I still prefer my modification. It's clearer and more straightforward, and the increase in code size is very small compared to the alternatives. Moreover, if we later want to implement 'cannot unload the last governor' (which might be the module reference approach you mentioned), we can simply remove this code."
> 
OK.  I'm fine with the above.
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
> 
>>
>> 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) {

Re: [PATCH v2 0/4] Fix some errors in the devfreq core layer when governor is NULL

[Jie Zhan]

On 4/1/2026 11:28 AM, Yaxiong Tian wrote:
> While doing some development work with devfreq_add_governor()/
> devfreq_remove_governor(), I discovered several bugs caused when
> devfreq->governor is NULL. Specifically:
> 
> 1) A possible null pointer issue in devfreq_add_governor(), caused
> by devfreq_remove_governor() setting devfreq->governor to NULL in
> certain situations, while devfreq_add_governor() lacks corresponding
> checks for devfreq->governor.
> 
> 2) When operating on governor and available_governors under /sys,
> there are also some unexpected errors.
> 
> For issue 1, the solution is similar to Jie Zhan's PATCH.
> https://lore.kernel.org/linux-pm/20260326123428.800407-6-zhanjie9@hisilicon.com/
> 
> The original v1 approach actually changed the original logic, which is incorrect.
> 
> See the following patches for details.
> 
> change in v2:
> 1) rebase to  linux-next(20260326) /devfreq-next 
> 2) remove some code to fix null pointer in patch1
> 3) add  sysfs_update_group() in patch2
> 
> 
> Yaxiong Tian (4):
>   PM / devfreq: Fix possible null pointer issue in
>     devfreq_add_governor()
>   PM / devfreq: Fix available_governors_show() when no governor is set
>   PM / devfreq: Fix governor_store() failing when device has no current
>     governor
>   PM / devfreq: Optimize error return value of governor_show()
> 
>  drivers/devfreq/devfreq.c | 57 +++++++++++----------------------------
>  1 file changed, 16 insertions(+), 41 deletions(-)
> 

Hi Yaxiong,

Any plan for updating this in the 7.1 cycle?

Jie

Re: [PATCH v2 0/4] Fix some errors in the devfreq core layer when governor is NULL

[Yaxiong Tian]

在 2026/4/28 10:50, Jie Zhan 写道:
>
> On 4/1/2026 11:28 AM, Yaxiong Tian wrote:
>> While doing some development work with devfreq_add_governor()/
>> devfreq_remove_governor(), I discovered several bugs caused when
>> devfreq->governor is NULL. Specifically:
>>
>> 1) A possible null pointer issue in devfreq_add_governor(), caused
>> by devfreq_remove_governor() setting devfreq->governor to NULL in
>> certain situations, while devfreq_add_governor() lacks corresponding
>> checks for devfreq->governor.
>>
>> 2) When operating on governor and available_governors under /sys,
>> there are also some unexpected errors.
>>
>> For issue 1, the solution is similar to Jie Zhan's PATCH.
>> https://lore.kernel.org/linux-pm/20260326123428.800407-6-zhanjie9@hisilicon.com/
>>
>> The original v1 approach actually changed the original logic, which is incorrect.
>>
>> See the following patches for details.
>>
>> change in v2:
>> 1) rebase to  linux-next(20260326) /devfreq-next
>> 2) remove some code to fix null pointer in patch1
>> 3) add  sysfs_update_group() in patch2
>>
>>
>> Yaxiong Tian (4):
>>    PM / devfreq: Fix possible null pointer issue in
>>      devfreq_add_governor()
>>    PM / devfreq: Fix available_governors_show() when no governor is set
>>    PM / devfreq: Fix governor_store() failing when device has no current
>>      governor
>>    PM / devfreq: Optimize error return value of governor_show()
>>
>>   drivers/devfreq/devfreq.c | 57 +++++++++++----------------------------
>>   1 file changed, 16 insertions(+), 41 deletions(-)
>>
> Hi Yaxiong,
>
> Any plan for updating this in the 7.1 cycle?
>
> Jie
No plans for now. I tried using BT4 and found that it can automatically 
extract your tags. Currently, we only have some disagreements on patch 
4. This patch isn't very important and can be ignored. Let's just wait 
for the maintainers to decide—maybe your solution or someone else's will 
be better.

Re: [PATCH v2 0/4] Fix some errors in the devfreq core layer when governor is NULL

[Jie Zhan]

On 4/28/2026 10:59 AM, Yaxiong Tian wrote:
> 
> 在 2026/4/28 10:50, Jie Zhan 写道:
>>
>> On 4/1/2026 11:28 AM, Yaxiong Tian wrote:
>>> While doing some development work with devfreq_add_governor()/
>>> devfreq_remove_governor(), I discovered several bugs caused when
>>> devfreq->governor is NULL. Specifically:
>>>
>>> 1) A possible null pointer issue in devfreq_add_governor(), caused
>>> by devfreq_remove_governor() setting devfreq->governor to NULL in
>>> certain situations, while devfreq_add_governor() lacks corresponding
>>> checks for devfreq->governor.
>>>
>>> 2) When operating on governor and available_governors under /sys,
>>> there are also some unexpected errors.
>>>
>>> For issue 1, the solution is similar to Jie Zhan's PATCH.
>>> https://lore.kernel.org/linux-pm/20260326123428.800407-6-zhanjie9@hisilicon.com/
>>>
>>> The original v1 approach actually changed the original logic, which is incorrect.
>>>
>>> See the following patches for details.
>>>
>>> change in v2:
>>> 1) rebase to  linux-next(20260326) /devfreq-next
>>> 2) remove some code to fix null pointer in patch1
>>> 3) add  sysfs_update_group() in patch2
>>>
>>>
>>> Yaxiong Tian (4):
>>>    PM / devfreq: Fix possible null pointer issue in
>>>      devfreq_add_governor()
>>>    PM / devfreq: Fix available_governors_show() when no governor is set
>>>    PM / devfreq: Fix governor_store() failing when device has no current
>>>      governor
>>>    PM / devfreq: Optimize error return value of governor_show()
>>>
>>>   drivers/devfreq/devfreq.c | 57 +++++++++++----------------------------
>>>   1 file changed, 16 insertions(+), 41 deletions(-)
>>>
>> Hi Yaxiong,
>>
>> Any plan for updating this in the 7.1 cycle?
>>
>> Jie
> No plans for now. I tried using BT4 and found that it can automatically extract your tags. Currently, we only have some disagreements on patch 4. This patch isn't very important and can be ignored. Let's just wait for the maintainers to decide—maybe your solution or someone else's will be better.
Sure.


Hi Chanwoo,

Can you take a look at Patch 1-3 and check if they can be applied?

We have some following patches in development that depends on this.

PS: Sorry I'm not sure which email address gets to you easier, so include
them all!

Thanks!
Jie


	

Re: [PATCH v2 1/4] PM / devfreq: Fix possible null pointer issue in devfreq_add_governor()

[Choi Chanwoo]

2026-04-01 PM 12:30에 Yaxiong Tian 이(가) 쓴 글:
> When a user removes a governor using devfreq_remove_governor(), if
> the current device is using this governor, devfreq->governor will
> be set to NULL. When the user registers any governor
> using devfreq_add_governor(), since devfreq->governor is NULL, a
> null pointer error occurs in strncmp().
> 
> For example: A user loads the userspace gov through a module, then
> a device selects userspace. When unloading the userspace module and
> then loading it again, the null pointer error occurs:
> 
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000010
> Mem abort info:
> ESR = 0x0000000096000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> *******************skip *********************
> Call trace:
> __pi_strncmp+0x20/0x1b8
> devfreq_userspace_init+0x1c/0xff8 [governor_userspace]
> do_one_initcall+0x4c/0x278
> do_init_module+0x5c/0x218
> load_module+0x1f1c/0x1fc8
> init_module_from_file+0x8c/0xd0
> __arm64_sys_finit_module+0x220/0x3d8
> invoke_syscall+0x48/0x110
> el0_svc_common.constprop.0+0xbc/0xe8
> do_el0_svc+0x20/0x30
> el0_svc+0x24/0xb8
> el0t_64_sync_handler+0xb8/0xc0
> el0t_64_sync+0x14c/0x150
> 
> To fix this issue, remove the list_for_each_entry() logic, as
> find_devfreq_governor() has already checked for the existence of
> governor with the same name. This makes it impossible to find a
> duplicate governor in the list, so the subsequent logic is
> unreachable and can be removed.
> 
> Fixes: 1b5c1be2c88e ("PM / devfreq: map devfreq drivers to governor using name")
> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
> ---
>  drivers/devfreq/devfreq.c | 33 ---------------------------------
>  1 file changed, 33 deletions(-)
> 
> Hi Jie Zhan: 
> 	If you're willing, I'd like to add your Co-developed-by tag.
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 82dd9a43dc62..994984f7b6e1 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1261,7 +1261,6 @@ void devfreq_resume(void)
>  int devfreq_add_governor(struct devfreq_governor *governor)
>  {
>  	struct devfreq_governor *g;
> -	struct devfreq *devfreq;
>  	int err = 0;
>  
>  	if (!governor) {
> @@ -1280,38 +1279,6 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>  
>  	list_add(&governor->node, &devfreq_governor_list);
>  
> -	list_for_each_entry(devfreq, &devfreq_list, node) {
> -		int ret = 0;
> -		struct device *dev = devfreq->dev.parent;
> -
> -		if (!strncmp(devfreq->governor->name, governor->name,
> -			     DEVFREQ_NAME_LEN)) {
> -			/* The following should never occur */
> -			if (devfreq->governor) {
> -				dev_warn(dev,
> -					 "%s: Governor %s already present\n",
> -					 __func__, devfreq->governor->name);
> -				ret = devfreq->governor->event_handler(devfreq,
> -							DEVFREQ_GOV_STOP, NULL);
> -				if (ret) {
> -					dev_warn(dev,
> -						 "%s: Governor %s stop = %d\n",
> -						 __func__,
> -						 devfreq->governor->name, ret);
> -				}
> -				/* Fall through */
> -			}
> -			devfreq->governor = governor;
> -			ret = devfreq->governor->event_handler(devfreq,
> -						DEVFREQ_GOV_START, NULL);
> -			if (ret) {
> -				dev_warn(dev, "%s: Governor %s start=%d\n",
> -					 __func__, devfreq->governor->name,
> -					 ret);
> -			}
> -		}
> -	}
> -
>  err_out:
>  	mutex_unlock(&devfreq_list_lock);
>  


Hi,

Applied it. Thanks for fixing it.

Thanks,
Chanwoo Choi

Re: [PATCH v2 2/4] PM / devfreq: Fix available_governors_show() when no governor is set

[Choi Chanwoo]

2026-04-01 PM 12:30에 Yaxiong Tian 이(가) 쓴 글:
> 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 use df->governor for validity
> verification in the following code.
> 
> Fixes: 483d557ee9a3 ("PM / devfreq: Clean up the devfreq instance name in sysfs attr")
> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
> ---
>  drivers/devfreq/devfreq.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 994984f7b6e1..2977b07be939 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1468,16 +1468,13 @@ static ssize_t available_governors_show(struct device *d,
>  	struct devfreq *df = to_devfreq(d);
>  	ssize_t count = 0;
>  
> -	if (!df->governor)
> -		return -EINVAL;
> -
>  	mutex_lock(&devfreq_list_lock);
>  
>  	/*
>  	 * The devfreq with immutable governor (e.g., passive) shows
>  	 * only own governor.
>  	 */
> -	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)) {
> +	if (df->governor && IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)) {
>  		count = scnprintf(&buf[count], DEVFREQ_NAME_LEN,
>  				  "%s ", df->governor->name);
>  	/*


Hi,

Applied it. Thanks

Thanks,
Chanwoo Choi

Re: [PATCH v2 4/4] PM / devfreq: Optimize error return value of governor_show()

[Choi Chanwoo]

2026-04-01 PM 12:31에 Yaxiong Tian 이(가) 쓴 글:
> When df->governor is NULL, governor_show() returns -EINVAL, which
> confuses users.
> 
> To fix this issue, return -ENOENT to indicate that no governor is
> currently set for the device.
> 
> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
> ---
>  drivers/devfreq/devfreq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 975f82d7a9d1..c40568d2a4dc 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1377,7 +1377,7 @@ static ssize_t governor_show(struct device *dev,
>  	struct devfreq *df = to_devfreq(dev);
>  
>  	if (!df->governor)
> -		return -EINVAL;
> +		return -ENOENT;
>  
>  	return sprintf(buf, "%s\n", df->governor->name);
>  }

Hi,

Applied it. Thanks,

Chanwoo Choi

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

[Choi Chanwoo]

2026-04-01 PM 12:31에 Yaxiong Tian 이(가) 쓴 글:
> 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@kylinos.cn>
> ---
>  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,

I think that the following patch is able to support on your case.
Could you please check it?

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index e5d3f9cf94dc..f08fc6966eae 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,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
                ret = PTR_ERR(governor);
                goto out;
        }
+       if (!df->governor)
+               goto start_new_governor;
+
        if (df->governor == governor) {
                ret = 0;
                goto out;
@@ -1423,6 +1423,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
                goto out;
        }
 
+start_new_governor:
        /*
         * Start the new governor and create the specific sysfs files
         * which depend on the new governor.
@@ -1436,6 +1437,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 
                /* Restore previous governor */
                df->governor = prev_governor;
+               if (!df->governor)
+                       goto out;
+
           

Thanks,
Chanwoo Choi

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

[Yaxiong Tian]

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")
Co-developed-by: Choi Chanwoo <cwchoi00@gmail.com>
Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
 drivers/devfreq/devfreq.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 2977b07be939..76e3700577ca 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,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 		ret = PTR_ERR(governor);
 		goto out;
 	}
+	if (!df->governor)
+		goto start_new_governor;
+
 	if (df->governor == governor) {
 		ret = 0;
 		goto out;
@@ -1423,6 +1423,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 		goto out;
 	}
 
+start_new_governor:
 	/*
 	 * Start the new governor and create the specific sysfs files
 	 * which depend on the new governor.
@@ -1436,6 +1437,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 
 		/* Restore previous governor */
 		df->governor = prev_governor;
+		if (!df->governor)
+			goto out;
+
 		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
 		if (ret) {
 			dev_err(dev,
-- 
2.25.1

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

[Yaxiong Tian]

在 2026/4/30 06:28, Choi Chanwoo 写道:
>
> 2026-04-01 PM 12:31에 Yaxiong Tian 이(가) 쓴 글:
>> 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@kylinos.cn>
>> ---
>>   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,
>
> I think that the following patch is able to support on your case.
> Could you please check it?
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index e5d3f9cf94dc..f08fc6966eae 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,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>                  ret = PTR_ERR(governor);
>                  goto out;
>          }
> +       if (!df->governor)
> +               goto start_new_governor;
> +
>          if (df->governor == governor) {
>                  ret = 0;
>                  goto out;
> @@ -1423,6 +1423,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>                  goto out;
>          }
>   
> +start_new_governor:
>          /*
>           * Start the new governor and create the specific sysfs files
>           * which depend on the new governor.
> @@ -1436,6 +1437,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   
>                  /* Restore previous governor */
>                  df->governor = prev_governor;
> +               if (!df->governor)
> +                       goto out;
> +
>             
>
> Thanks,
> Chanwoo Choi
Of course. I have tested it, and this patch is fine. So if you have 
already prepared
the patch, you can add my Reviewed-by tag:
Reviewed-by: Yaxiong Tian tianyaxiong@kylinos.cn.
Or directly use my V3 version:
https://lore.kernel.org/all/20260430032035.75496-1-tianyaxiong@kylinos.cn/