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, modify the relevant logic in devfreq_add_governor():
Only check whether the new governor matches the existing one when
devfreq->governor exists. When devfreq->governor is NULL, directly
select the new governor and perform the DEVFREQ_GOV_START operation.

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

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 54f0b18536db..63ce6e25abe2 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1288,23 +1288,21 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 		int ret = 0;
 		struct device *dev = devfreq->dev.parent;
 
-		if (!strncmp(devfreq->governor->name, governor->name,
+		if (devfreq->governor && !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 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 */
+					 "%s: Governor %s stop = %d\n",
+					 __func__,
+					 devfreq->governor->name, ret);
 			}
+		} else if (!devfreq->governor) {
 			devfreq->governor = governor;
 			ret = devfreq->governor->event_handler(devfreq,
 						DEVFREQ_GOV_START, NULL);
-- 
2.25.1

On 3/19/2026 5:16 PM, 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, modify the relevant logic in devfreq_add_governor():
> Only check whether the new governor matches the existing one when
> devfreq->governor exists. When devfreq->governor is NULL, directly
> select the new governor and perform the DEVFREQ_GOV_START operation.
> 
> Fixes: 1b5c1be2c88e ("PM / devfreq: map devfreq drivers to governor using name")
> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
> ---
>  drivers/devfreq/devfreq.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 54f0b18536db..63ce6e25abe2 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1288,23 +1288,21 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>  		int ret = 0;
>  		struct device *dev = devfreq->dev.parent;
>  
> -		if (!strncmp(devfreq->governor->name, governor->name,
> +		if (devfreq->governor && !strncmp(devfreq->governor->name, governor->name,
>  			     DEVFREQ_NAME_LEN)) {

Probaly do:
if (!devfreq->governor)
	continue;
so as to keep the line short and the "else if (!devfreq->governor) {" below
can be removed.

and also:
if (!strncmp(devfreq->governor->name, governor->name,
	DEVFREQ_NAME_LEN))
	continue;
so we don't have to indent that much.

>  			/* 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 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 */
> +					 "%s: Governor %s stop = %d\n",
> +					 __func__,
> +					 devfreq->governor->name, ret);
>  			}
> +		} else if (!devfreq->governor) {
>  			devfreq->governor = governor;
>  			ret = devfreq->governor->event_handler(devfreq,
>  						DEVFREQ_GOV_START, NULL);

在 2026/3/31 15:04, Jie Zhan 写道:
>
> On 3/19/2026 5:16 PM, 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, modify the relevant logic in devfreq_add_governor():
>> Only check whether the new governor matches the existing one when
>> devfreq->governor exists. When devfreq->governor is NULL, directly
>> select the new governor and perform the DEVFREQ_GOV_START operation.
>>
>> Fixes: 1b5c1be2c88e ("PM / devfreq: map devfreq drivers to governor using name")
>> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
>> ---
>>   drivers/devfreq/devfreq.c | 24 +++++++++++-------------
>>   1 file changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 54f0b18536db..63ce6e25abe2 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -1288,23 +1288,21 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>>   		int ret = 0;
>>   		struct device *dev = devfreq->dev.parent;
>>   
>> -		if (!strncmp(devfreq->governor->name, governor->name,
>> +		if (devfreq->governor && !strncmp(devfreq->governor->name, governor->name,
>>   			     DEVFREQ_NAME_LEN)) {
> Probaly do:
> if (!devfreq->governor)
> 	continue;
> so as to keep the line short and the "else if (!devfreq->governor) {" below
> can be removed.
If that's the case, a dev without a governor would not be able to add a 
new governor.
>
> and also:
> if (!strncmp(devfreq->governor->name, governor->name,
> 	DEVFREQ_NAME_LEN))
> 	continue;
> so we don't have to indent that much.

Currently, governor->name can be set arbitrarily, so duplicate names are 
possible.

But they could still be two different governors. Leaving it unchanged 
seems fine to me.

>
>>   			/* 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 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 */
>> +					 "%s: Governor %s stop = %d\n",
>> +					 __func__,
>> +					 devfreq->governor->name, ret);
>>   			}
>> +		} else if (!devfreq->governor) {
>>   			devfreq->governor = governor;
>>   			ret = devfreq->governor->event_handler(devfreq,
>>   						DEVFREQ_GOV_START, NULL);

On 3/31/2026 3:49 PM, Yaxiong Tian wrote:
> 
> 在 2026/3/31 15:04, Jie Zhan 写道:
>>
>> On 3/19/2026 5:16 PM, 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, modify the relevant logic in devfreq_add_governor():
>>> Only check whether the new governor matches the existing one when
>>> devfreq->governor exists. When devfreq->governor is NULL, directly
>>> select the new governor and perform the DEVFREQ_GOV_START operation.
>>>
>>> Fixes: 1b5c1be2c88e ("PM / devfreq: map devfreq drivers to governor using name")
>>> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
>>> ---
>>>   drivers/devfreq/devfreq.c | 24 +++++++++++-------------
>>>   1 file changed, 11 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index 54f0b18536db..63ce6e25abe2 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -1288,23 +1288,21 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>>>           int ret = 0;
>>>           struct device *dev = devfreq->dev.parent;
>>>   -        if (!strncmp(devfreq->governor->name, governor->name,
>>> +        if (devfreq->governor && !strncmp(devfreq->governor->name, governor->name,
>>>                    DEVFREQ_NAME_LEN)) {
>> Probaly do:
>> if (!devfreq->governor)
>>     continue;
>> so as to keep the line short and the "else if (!devfreq->governor) {" below
>> can be removed.
> If that's the case, a dev without a governor would not be able to add a new governor.
It's not about adding.

The whole point here (the block inside list_for_each_entry()) is a very
rare safe check that you're adding a governor of the same name of a
governor used by an existing devfreq device.  If so, stop the old one and
start the new one.

Therefore, it doesn't have to start/restart it if devfreq->governor is
NULL.
>>
>> and also:
>> if (!strncmp(devfreq->governor->name, governor->name,
>>     DEVFREQ_NAME_LEN))
>>     continue;
>> so we don't have to indent that much.
> 
> Currently, governor->name can be set arbitrarily, so duplicate names are possible.
> 
> But they could still be two different governors. Leaving it unchanged seems fine to me.
> 
Sorry, my mistake!

I mean:
if (strncmp(devfreq->governor->name, governor->name,
	DEVFREQ_NAME_LEN))
	continue;

>>
>>>               /* 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 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 */
>>> +                     "%s: Governor %s stop = %d\n",
>>> +                     __func__,
>>> +                     devfreq->governor->name, ret);
>>>               }
>>> +        } else if (!devfreq->governor) {
>>>               devfreq->governor = governor;
>>>               ret = devfreq->governor->event_handler(devfreq,
>>>                           DEVFREQ_GOV_START, NULL);

在 2026/3/31 16:49, Jie Zhan 写道:
>
> On 3/31/2026 3:49 PM, Yaxiong Tian wrote:
>> 在 2026/3/31 15:04, Jie Zhan 写道:
>>> On 3/19/2026 5:16 PM, 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, modify the relevant logic in devfreq_add_governor():
>>>> Only check whether the new governor matches the existing one when
>>>> devfreq->governor exists. When devfreq->governor is NULL, directly
>>>> select the new governor and perform the DEVFREQ_GOV_START operation.
>>>>
>>>> Fixes: 1b5c1be2c88e ("PM / devfreq: map devfreq drivers to governor using name")
>>>> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
>>>> ---
>>>>    drivers/devfreq/devfreq.c | 24 +++++++++++-------------
>>>>    1 file changed, 11 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>> index 54f0b18536db..63ce6e25abe2 100644
>>>> --- a/drivers/devfreq/devfreq.c
>>>> +++ b/drivers/devfreq/devfreq.c
>>>> @@ -1288,23 +1288,21 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>>>>            int ret = 0;
>>>>            struct device *dev = devfreq->dev.parent;
>>>>    -        if (!strncmp(devfreq->governor->name, governor->name,
>>>> +        if (devfreq->governor && !strncmp(devfreq->governor->name, governor->name,
>>>>                     DEVFREQ_NAME_LEN)) {
>>> Probaly do:
>>> if (!devfreq->governor)
>>>      continue;
>>> so as to keep the line short and the "else if (!devfreq->governor) {" below
>>> can be removed.
>> If that's the case, a dev without a governor would not be able to add a new governor.
> It's not about adding.
>
> The whole point here (the block inside list_for_each_entry()) is a very
> rare safe check that you're adding a governor of the same name of a
> governor used by an existing devfreq device.  If so, stop the old one and
> start the new one.
>
> Therefore, it doesn't have to start/restart it if devfreq->governor is
> NULL.
In the current patch, it directly start when devfreq->governor is NULL, 
and only follows the same-name logic you mentioned when it is non-NULL.

There isn't the issue you mentioned.

>>> and also:
>>> if (!strncmp(devfreq->governor->name, governor->name,
>>>      DEVFREQ_NAME_LEN))
>>>      continue;
>>> so we don't have to indent that much.
>> Currently, governor->name can be set arbitrarily, so duplicate names are possible.
>>
>> But they could still be two different governors. Leaving it unchanged seems fine to me.
>>
> Sorry, my mistake!
>
> I mean:
> if (strncmp(devfreq->governor->name, governor->name,
> 	DEVFREQ_NAME_LEN))
> 	continue;
>
>>>>                /* 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 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 */
>>>> +                     "%s: Governor %s stop = %d\n",
>>>> +                     __func__,
>>>> +                     devfreq->governor->name, ret);
>>>>                }
>>>> +        } else if (!devfreq->governor) {
>>>>                devfreq->governor = governor;
>>>>                ret = devfreq->governor->event_handler(devfreq,
>>>>                            DEVFREQ_GOV_START, NULL);

On 3/31/2026 5:17 PM, Yaxiong Tian wrote:
> 
> 在 2026/3/31 16:49, Jie Zhan 写道:
>>
>> On 3/31/2026 3:49 PM, Yaxiong Tian wrote:
>>> 在 2026/3/31 15:04, Jie Zhan 写道:
>>>> On 3/19/2026 5:16 PM, 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, modify the relevant logic in devfreq_add_governor():
>>>>> Only check whether the new governor matches the existing one when
>>>>> devfreq->governor exists. When devfreq->governor is NULL, directly
>>>>> select the new governor and perform the DEVFREQ_GOV_START operation.
>>>>>
>>>>> Fixes: 1b5c1be2c88e ("PM / devfreq: map devfreq drivers to governor using name")
>>>>> Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
>>>>> ---
>>>>>    drivers/devfreq/devfreq.c | 24 +++++++++++-------------
>>>>>    1 file changed, 11 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>>> index 54f0b18536db..63ce6e25abe2 100644
>>>>> --- a/drivers/devfreq/devfreq.c
>>>>> +++ b/drivers/devfreq/devfreq.c
>>>>> @@ -1288,23 +1288,21 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>>>>>            int ret = 0;
>>>>>            struct device *dev = devfreq->dev.parent;
>>>>>    -        if (!strncmp(devfreq->governor->name, governor->name,
>>>>> +        if (devfreq->governor && !strncmp(devfreq->governor->name, governor->name,
>>>>>                     DEVFREQ_NAME_LEN)) {
>>>> Probaly do:
>>>> if (!devfreq->governor)
>>>>      continue;
>>>> so as to keep the line short and the "else if (!devfreq->governor) {" below
>>>> can be removed.
>>> If that's the case, a dev without a governor would not be able to add a new governor.
>> It's not about adding.
>>
>> The whole point here (the block inside list_for_each_entry()) is a very
>> rare safe check that you're adding a governor of the same name of a
>> governor used by an existing devfreq device.  If so, stop the old one and
>> start the new one.
>>
>> Therefore, it doesn't have to start/restart it if devfreq->governor is
>> NULL.
> In the current patch, it directly start when devfreq->governor is NULL, and only follows the same-name logic you mentioned when it is non-NULL.
> 
> There isn't the issue you mentioned.
> 
Ok, so create_sysfs_files() is skipped...
For example, setting 'userspace' as the governor won't create its belonging
files.

I'd recommend sticking with the existing logic here and avoid adding a new
block."

>>>> and also:
>>>> if (!strncmp(devfreq->governor->name, governor->name,
>>>>      DEVFREQ_NAME_LEN))
>>>>      continue;
>>>> so we don't have to indent that much.
>>> Currently, governor->name can be set arbitrarily, so duplicate names are possible.
>>>
>>> But they could still be two different governors. Leaving it unchanged seems fine to me.
>>>
>> Sorry, my mistake!
>>
>> I mean:
>> if (strncmp(devfreq->governor->name, governor->name,
>>     DEVFREQ_NAME_LEN))
>>     continue;
>>
>>>>>                /* 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 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 */
>>>>> +                     "%s: Governor %s stop = %d\n",
>>>>> +                     __func__,
>>>>> +                     devfreq->governor->name, ret);
>>>>>                }
>>>>> +        } else if (!devfreq->governor) {
>>>>>                devfreq->governor = governor;
>>>>>                ret = devfreq->governor->event_handler(devfreq,
>>>>>                            DEVFREQ_GOV_START, NULL);