| Message ID | 20230826095201.1137288-1-liaochang1@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | cpufreq: Fix inconsistency in error messages of cpufreq_resume/suspend() | expand |
On 26-08-23, 09:52, Liao Chang wrote: > The error message printed by cpufreq_resume() currently is the pointer > value of the policy structure, while the error message printed by > cpufreq_suspend() is the name of the driver. In order to make the error > messages more consistent and easier to understand, change the error > message printed by cpufreq_resume() to the name of driver. Also I don't think printing kernel addresses will help much anyway, and it also may not be recommended. > Signed-off-by: Liao Chang <liaochang1@huawei.com> > --- > drivers/cpufreq/cpufreq.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index c835ff117386..2199c04ac64d 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -1943,8 +1943,8 @@ void cpufreq_resume(void) > > for_each_active_policy(policy) { > if (cpufreq_driver->resume && cpufreq_driver->resume(policy)) { > - pr_err("%s: Failed to resume driver: %p\n", __func__, > - policy); > + pr_err("%s: Failed to resume driver: %s\n", __func__, > + cpufreq_driver->name); > } else if (has_target()) { > down_write(&policy->rwsem); > ret = cpufreq_start_governor(policy); There is another print down here which prints policy, please update that too in a similar way.
Hi, Viresh 在 2023/8/28 15:00, Viresh Kumar 写道: > On 26-08-23, 09:52, Liao Chang wrote: >> The error message printed by cpufreq_resume() currently is the pointer >> value of the policy structure, while the error message printed by >> cpufreq_suspend() is the name of the driver. In order to make the error >> messages more consistent and easier to understand, change the error >> message printed by cpufreq_resume() to the name of driver. > > Also I don't think printing kernel addresses will help much anyway, > and it also may not be recommended. > >> Signed-off-by: Liao Chang <liaochang1@huawei.com> >> --- >> drivers/cpufreq/cpufreq.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >> index c835ff117386..2199c04ac64d 100644 >> --- a/drivers/cpufreq/cpufreq.c >> +++ b/drivers/cpufreq/cpufreq.c >> @@ -1943,8 +1943,8 @@ void cpufreq_resume(void) >> >> for_each_active_policy(policy) { >> if (cpufreq_driver->resume && cpufreq_driver->resume(policy)) { >> - pr_err("%s: Failed to resume driver: %p\n", __func__, >> - policy); >> + pr_err("%s: Failed to resume driver: %s\n", __func__, >> + cpufreq_driver->name); >> } else if (has_target()) { >> down_write(&policy->rwsem); >> ret = cpufreq_start_governor(policy); > > There is another print down here which prints policy, please update > that too in a similar way. What about printing message like this below when cpufreq_start_governor() fails. diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 50bbc969ffe5..b78b509429a6 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1943,16 +1943,16 @@ void cpufreq_resume(void) if (ret) - pr_err("%s: Failed to start governor for policy: %p\n", - __func__, policy); + pr_err("%s: Failed to start governor for policy%u\n", + __func__, policy->cpu); } } } >
On 28-08-23, 20:31, Liao, Chang wrote: > What about printing message like this below when cpufreq_start_governor() fails. > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 50bbc969ffe5..b78b509429a6 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -1943,16 +1943,16 @@ void cpufreq_resume(void) > > if (ret) > - pr_err("%s: Failed to start governor for policy: %p\n", > - __func__, policy); > + pr_err("%s: Failed to start governor for policy%u\n", s/policy/CPU%u's policy/ ? > + __func__, policy->cpu); Sounds good.
在 2023/8/29 14:01, Viresh Kumar 写道: > On 28-08-23, 20:31, Liao, Chang wrote: >> What about printing message like this below when cpufreq_start_governor() fails. >> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >> index 50bbc969ffe5..b78b509429a6 100644 >> --- a/drivers/cpufreq/cpufreq.c >> +++ b/drivers/cpufreq/cpufreq.c >> @@ -1943,16 +1943,16 @@ void cpufreq_resume(void) >> >> if (ret) >> - pr_err("%s: Failed to start governor for policy: %p\n", >> - __func__, policy); >> + pr_err("%s: Failed to start governor for policy%u\n", > > s/policy/CPU%u's policy/ ? Sounds good, thanks.
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index c835ff117386..2199c04ac64d 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1943,8 +1943,8 @@ void cpufreq_resume(void) for_each_active_policy(policy) { if (cpufreq_driver->resume && cpufreq_driver->resume(policy)) { - pr_err("%s: Failed to resume driver: %p\n", __func__, - policy); + pr_err("%s: Failed to resume driver: %s\n", __func__, + cpufreq_driver->name); } else if (has_target()) { down_write(&policy->rwsem); ret = cpufreq_start_governor(policy);
The error message printed by cpufreq_resume() currently is the pointer value of the policy structure, while the error message printed by cpufreq_suspend() is the name of the driver. In order to make the error messages more consistent and easier to understand, change the error message printed by cpufreq_resume() to the name of driver. Signed-off-by: Liao Chang <liaochang1@huawei.com> --- drivers/cpufreq/cpufreq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)