| Message ID | 1444837943-32263-1-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
Rafael, On Wed, 14 Oct 2015 23:28:51 +0200, Rafael J. Wysocki wrote: > > To avoid this confusion, this patch simply deletes the error message. > > > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > That's only present in linux-next now, right? No, it is already in 4.3-rc since 4.3-rc1. The commit adding the problematic function is 8d4d4e98acd68c31435ebb7beea591dbf60b9eb2, and this function gets used in cpufreq-dt starting at commit 2e02d8723edf6599988852a8ade8f83b2f766cb8. $ git tag --contains 2e02d8723edf6599988852a8ade8f83b2f766cb8 v4.3-rc1 v4.3-rc2 v4.3-rc3 v4.3-rc4 v4.3-rc5 Best regards, Thomas
On Wednesday, October 14, 2015 05:52:23 PM Thomas Petazzoni wrote: > The of_cpumask_init_opp_table() function will print an error message > (with pr_err) if it cannot find the OPP table for a certain CPU in the > Device Tree. > > There are users of the cpufreq-dt driver (which is the one calling > of_cpumask_init_opp_table) that do not have the OPP points defined in > the Device Tree. Instead, such users dynamically create the OPP table > at boot time depending on the system configuration. Such a case is > planned in the cpufreq-dt driver, which on purpose ignores the return > value of of_cpumask_init_opp_table() with the following comment: "OPPs > might be populated at runtime, don't check for error here". > > For such platforms, the of_cpumask_init_opp_table() prints a spurious > and confusing error message for each CPU: > > [ 1.749548] of_cpumask_init_opp_table: couldn't find opp table for cpu:0, -19 > [ 1.756784] of_cpumask_init_opp_table: couldn't find opp table for cpu:1, -19 > [ 1.764031] of_cpumask_init_opp_table: couldn't find opp table for cpu:2, -19 > [ 1.771268] of_cpumask_init_opp_table: couldn't find opp table for cpu:3, -19 > > This is confusing because everything is working fine, cpufreq works > and it knows the OPP table that was registered at boot time (on > Marvell Armada XP): > > $ cat /sys/devices/system/cpu/cpu0/cpufreq/stats/time_in_state > 666500 1884 > 1333000 23333 > > To avoid this confusion, this patch simply deletes the error message. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> That's only present in linux-next now, right? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Wed, 14 Oct 2015 23:44:03 +0200, Rafael J. Wysocki wrote: > On Wednesday, October 14, 2015 11:10:39 PM Thomas Petazzoni wrote: > > Rafael, > > > > On Wed, 14 Oct 2015 23:28:51 +0200, Rafael J. Wysocki wrote: > > > > > > To avoid this confusion, this patch simply deletes the error message. > > > > > > > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > > > > > That's only present in linux-next now, right? > > > > No, it is already in 4.3-rc since 4.3-rc1. The commit adding the > > problematic function is 8d4d4e98acd68c31435ebb7beea591dbf60b9eb2, and > > this function gets used in cpufreq-dt starting at commit > > 2e02d8723edf6599988852a8ade8f83b2f766cb8. > > > > $ git tag --contains 2e02d8723edf6599988852a8ade8f83b2f766cb8 > > v4.3-rc1 > > v4.3-rc2 > > v4.3-rc3 > > v4.3-rc4 > > v4.3-rc5 > > OK, thanks! > > It should carry a > > Fixes: 2e02d8723edf > > tag then, right? Yes, I wasn't sure if it really is a fix, and if so, which commit it is fixing. It is really 8d4d4e98acd68c31435ebb7beea591dbf60b9eb2 that introduces the confusing message, but the message will not appear until the newly added function gets used in 2e02d8723edf6599988852a8ade8f83b2f766cb8. But if 2e02d8723edf6599988852a8ade8f83b2f766cb8 is considered to be the commit actually introducing the regression, then I'll resubmit an updated version with the Fixes: tag. Thanks! Thomas
Rafael, On Wed, 14 Oct 2015 23:59:45 +0200, Rafael J. Wysocki wrote: > > Yes, I wasn't sure if it really is a fix, and if so, which commit it is > > fixing. It is really 8d4d4e98acd68c31435ebb7beea591dbf60b9eb2 that > > introduces the confusing message, but the message will not appear until > > the newly added function gets used in > > 2e02d8723edf6599988852a8ade8f83b2f766cb8. > > > > But if 2e02d8723edf6599988852a8ade8f83b2f766cb8 is considered to be the > > commit actually introducing the regression, then I'll resubmit an > > updated version with the Fixes: tag. > > I've added the tag and applied the patch already. Awesome, thanks! Thomas
On Wednesday, October 14, 2015 11:10:39 PM Thomas Petazzoni wrote: > Rafael, > > On Wed, 14 Oct 2015 23:28:51 +0200, Rafael J. Wysocki wrote: > > > > To avoid this confusion, this patch simply deletes the error message. > > > > > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > > > That's only present in linux-next now, right? > > No, it is already in 4.3-rc since 4.3-rc1. The commit adding the > problematic function is 8d4d4e98acd68c31435ebb7beea591dbf60b9eb2, and > this function gets used in cpufreq-dt starting at commit > 2e02d8723edf6599988852a8ade8f83b2f766cb8. > > $ git tag --contains 2e02d8723edf6599988852a8ade8f83b2f766cb8 > v4.3-rc1 > v4.3-rc2 > v4.3-rc3 > v4.3-rc4 > v4.3-rc5 OK, thanks! It should carry a Fixes: 2e02d8723edf tag then, right? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, October 14, 2015 11:29:27 PM Thomas Petazzoni wrote: > Hello, > > On Wed, 14 Oct 2015 23:44:03 +0200, Rafael J. Wysocki wrote: > > On Wednesday, October 14, 2015 11:10:39 PM Thomas Petazzoni wrote: > > > Rafael, > > > > > > On Wed, 14 Oct 2015 23:28:51 +0200, Rafael J. Wysocki wrote: > > > > > > > > To avoid this confusion, this patch simply deletes the error message. > > > > > > > > > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > > > > > > > That's only present in linux-next now, right? > > > > > > No, it is already in 4.3-rc since 4.3-rc1. The commit adding the > > > problematic function is 8d4d4e98acd68c31435ebb7beea591dbf60b9eb2, and > > > this function gets used in cpufreq-dt starting at commit > > > 2e02d8723edf6599988852a8ade8f83b2f766cb8. > > > > > > $ git tag --contains 2e02d8723edf6599988852a8ade8f83b2f766cb8 > > > v4.3-rc1 > > > v4.3-rc2 > > > v4.3-rc3 > > > v4.3-rc4 > > > v4.3-rc5 > > > > OK, thanks! > > > > It should carry a > > > > Fixes: 2e02d8723edf > > > > tag then, right? > > Yes, I wasn't sure if it really is a fix, and if so, which commit it is > fixing. It is really 8d4d4e98acd68c31435ebb7beea591dbf60b9eb2 that > introduces the confusing message, but the message will not appear until > the newly added function gets used in > 2e02d8723edf6599988852a8ade8f83b2f766cb8. > > But if 2e02d8723edf6599988852a8ade8f83b2f766cb8 is considered to be the > commit actually introducing the regression, then I'll resubmit an > updated version with the Fixes: tag. I've added the tag and applied the patch already. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14-10-15, 17:52, Thomas Petazzoni wrote: > ret = of_init_opp_table(cpu_dev); > if (ret) { > - pr_err("%s: couldn't find opp table for cpu:%d, %d\n", > - __func__, cpu, ret); > - It should have be modified to pr_debug() at least.
Viresh, Rafael, On Thu, 15 Oct 2015 13:05:44 +0530, Viresh Kumar wrote: > On 14-10-15, 17:52, Thomas Petazzoni wrote: > > ret = of_init_opp_table(cpu_dev); > > if (ret) { > > - pr_err("%s: couldn't find opp table for cpu:%d, %d\n", > > - __func__, cpu, ret); > > - > > It should have be modified to pr_debug() at least. Would be fine as well, but then maybe with a rewording: pr_debug("%s: no static opp table found for cpu %d, assuming one will be added at runtime (%d)\n", __func__, cpu, ret); Do you want me to resend a new version? Or a follow-up patch based on the latest tree from Rafael now that he has applied the previous version? Thomas
On 15-10-15, 09:42, Thomas Petazzoni wrote: > Do you want me to resend a new version? Or a follow-up patch based on > the latest tree from Rafael now that he has applied the previous version? A follow up patch, without any Fixes tag would be fine.
On Thursday, October 15, 2015 08:07:38 PM Viresh Kumar wrote: > On 15-10-15, 09:42, Thomas Petazzoni wrote: > > Do you want me to resend a new version? Or a follow-up patch based on > > the latest tree from Rafael now that he has applied the previous version? > > A follow up patch, without any Fixes tag would be fine. Well, to be honest, I would prefer a replacement. And since it is going to miss -rc6 anyway, I'll probably simply let it wait for the v4.4 merge window. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15-10-15, 23:26, Rafael J. Wysocki wrote:
> Well, to be honest, I would prefer a replacement.
Ahh, I thought you will suggest a separate patch so that you don't
need to rebase your tree. But I don't have any issues with a new
version of the patch.
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 7ae7cd9..11206d2 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -1489,9 +1489,6 @@ int of_cpumask_init_opp_table(cpumask_var_t cpumask) ret = of_init_opp_table(cpu_dev); if (ret) { - pr_err("%s: couldn't find opp table for cpu:%d, %d\n", - __func__, cpu, ret); - /* Free all other OPPs */ of_cpumask_free_opp_table(cpumask); break;
The of_cpumask_init_opp_table() function will print an error message (with pr_err) if it cannot find the OPP table for a certain CPU in the Device Tree. There are users of the cpufreq-dt driver (which is the one calling of_cpumask_init_opp_table) that do not have the OPP points defined in the Device Tree. Instead, such users dynamically create the OPP table at boot time depending on the system configuration. Such a case is planned in the cpufreq-dt driver, which on purpose ignores the return value of of_cpumask_init_opp_table() with the following comment: "OPPs might be populated at runtime, don't check for error here". For such platforms, the of_cpumask_init_opp_table() prints a spurious and confusing error message for each CPU: [ 1.749548] of_cpumask_init_opp_table: couldn't find opp table for cpu:0, -19 [ 1.756784] of_cpumask_init_opp_table: couldn't find opp table for cpu:1, -19 [ 1.764031] of_cpumask_init_opp_table: couldn't find opp table for cpu:2, -19 [ 1.771268] of_cpumask_init_opp_table: couldn't find opp table for cpu:3, -19 This is confusing because everything is working fine, cpufreq works and it knows the OPP table that was registered at boot time (on Marvell Armada XP): $ cat /sys/devices/system/cpu/cpu0/cpufreq/stats/time_in_state 666500 1884 1333000 23333 To avoid this confusion, this patch simply deletes the error message. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/base/power/opp.c | 3 --- 1 file changed, 3 deletions(-)