Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Ubuntu Kernel Team (http://www.linux-archive.org/ubuntu-kernel-team/)
-   -   OMAP3630: PM: don't warn the user with a trace in case of PM34XX_ERRATUM (http://www.linux-archive.org/ubuntu-kernel-team/482454-omap3630-pm-dont-warn-user-trace-case-pm34xx_erratum.html)

Ricardo Salveti de Araujo 01-28-2011 04:17 PM

OMAP3630: PM: don't warn the user with a trace in case of PM34XX_ERRATUM
 
In case in user has a OMAP3630 < ES1.2 the kernel should warn the user
about the ERRATUM, but using pr_warning instead of WARN_ON is already
enough, as there is nothing else the user can do besides changing the
board.

With this we avoid having the following calltrace while booting with some
Beagle xM revisions:
WARNING: at /home/apw/build/ubuntu-natty2/ubuntu-natty2/arch/arm/mach-omap2/cpuidle34xx.c:468 omap_init_power_states+0x230/0x238()
omap_init_power_states: core off state C7 disabled due to i583
Modules linked in:
[<c00596f4>] (unwind_backtrace+0x0/0xfc) from [<c04f29a8>] (dump_stack+0x18/0x1c)
[<c04f29a8>] (dump_stack+0x18/0x1c) from [<c008510c>] (warn_slowpath_common+0x5c/0x6c)
[<c008510c>] (warn_slowpath_common+0x5c/0x6c) from [<c00851c0>] (warn_slowpath_fmt+0x38/0x40)
[<c00851c0>] (warn_slowpath_fmt+0x38/0x40) from [<c00676f4>] (omap_init_power_states+0x230/0x238)
[<c00676f4>] (omap_init_power_states+0x230/0x238) from [<c00131a0>] (omap3_idle_init+0x74/0x18c)
[<c00131a0>] (omap3_idle_init+0x74/0x18c) from [<c00126b4>] (omap3_pm_init+0x1ac/0x308)
[<c00126b4>] (omap3_pm_init+0x1ac/0x308) from [<c00474c0>] (do_one_initcall+0x3c/0x1b4)
[<c00474c0>] (do_one_initcall+0x3c/0x1b4) from [<c0008d58>] (kernel_init+0xe0/0x178)
[<c0008d58>] (kernel_init+0xe0/0x178) from [<c00532c8>] (kernel_thread_exit+0x0/0x8)
---[ end trace e639b107cbbc60f1 ]---

Signed-off-by: Ricardo Salveti de Araujo <ricardo.salveti@canonical.com>
---
arch/arm/mach-omap2/cpuidle34xx.c | 2 +-
arch/arm/mach-omap2/pm34xx.c | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index f7b22a1..54ec28e 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -464,7 +464,7 @@ void omap_init_power_states(void)
if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {
omap3_power_states[OMAP3_STATE_C7].valid = 0;
cpuidle_params_table[OMAP3_STATE_C7].valid = 0;
- WARN_ONCE(1, "%s: core off state C7 disabled due to i583
",
+ pr_warning("%s: core off state C7 disabled due to i583
",
__func__);
}
}
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 8cbbeade..d1ad1c1 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -927,8 +927,7 @@ void omap3_pm_off_mode_enable(int enable)
pwrst->pwrdm == core_pwrdm &&
state == PWRDM_POWER_OFF) {
pwrst->next_state = PWRDM_POWER_RET;
- WARN_ONCE(1,
- "%s: Core OFF disabled due to errata i583
",
+ pr_warning("%s: Core OFF disabled due to errata i583
",
__func__);
} else {
pwrst->next_state = state;
--
1.7.2.3


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Bryan Wu 01-30-2011 11:04 AM

OMAP3630: PM: don't warn the user with a trace in case of PM34XX_ERRATUM
 
On Sat, Jan 29, 2011 at 4:42 AM, Tim Gardner <tim.gardner@canonical.com> wrote:
> On 01/28/2011 10:17 AM, Ricardo Salveti de Araujo wrote:
>>
>> In case in user has a OMAP3630< *ES1.2 the kernel should warn the user
>> about the ERRATUM, but using pr_warning instead of WARN_ON is already
>> enough, as there is nothing else the user can do besides changing the
>> board.
>>
>> With this we avoid having the following calltrace while booting with some
>> Beagle xM revisions:
>> WARNING: at
>> /home/apw/build/ubuntu-natty2/ubuntu-natty2/arch/arm/mach-omap2/cpuidle34xx.c:468
>> omap_init_power_states+0x230/0x238()
>> omap_init_power_states: core off state C7 disabled due to i583
>> Modules linked in:
>> [<c00596f4>] (unwind_backtrace+0x0/0xfc) from [<c04f29a8>]
>> (dump_stack+0x18/0x1c)
>> [<c04f29a8>] (dump_stack+0x18/0x1c) from [<c008510c>]
>> (warn_slowpath_common+0x5c/0x6c)
>> [<c008510c>] (warn_slowpath_common+0x5c/0x6c) from [<c00851c0>]
>> (warn_slowpath_fmt+0x38/0x40)
>> [<c00851c0>] (warn_slowpath_fmt+0x38/0x40) from [<c00676f4>]
>> (omap_init_power_states+0x230/0x238)
>> [<c00676f4>] (omap_init_power_states+0x230/0x238) from [<c00131a0>]
>> (omap3_idle_init+0x74/0x18c)
>> [<c00131a0>] (omap3_idle_init+0x74/0x18c) from [<c00126b4>]
>> (omap3_pm_init+0x1ac/0x308)
>> [<c00126b4>] (omap3_pm_init+0x1ac/0x308) from [<c00474c0>]
>> (do_one_initcall+0x3c/0x1b4)
>> [<c00474c0>] (do_one_initcall+0x3c/0x1b4) from [<c0008d58>]
>> (kernel_init+0xe0/0x178)
>> [<c0008d58>] (kernel_init+0xe0/0x178) from [<c00532c8>]
>> (kernel_thread_exit+0x0/0x8)
>> ---[ end trace e639b107cbbc60f1 ]---
>>
>> Signed-off-by: Ricardo Salveti de Araujo<ricardo.salveti@canonical.com>
>> ---
>> *arch/arm/mach-omap2/cpuidle34xx.c | * *2 +-
>> *arch/arm/mach-omap2/pm34xx.c * * *| * *3 +--
>> *2 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
>> b/arch/arm/mach-omap2/cpuidle34xx.c
>> index f7b22a1..54ec28e 100644
>> --- a/arch/arm/mach-omap2/cpuidle34xx.c
>> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
>> @@ -464,7 +464,7 @@ void omap_init_power_states(void)
>> * * * *if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {
>> * * * * * * * *omap3_power_states[OMAP3_STATE_C7].valid = 0;
>> * * * * * * * *cpuidle_params_table[OMAP3_STATE_C7].valid = 0;
>> - * * * * * * * WARN_ONCE(1, "%s: core off state C7 disabled due to
>> i583
",
>> + * * * * * * * pr_warning("%s: core off state C7 disabled due to i583
",
>> * * * * * * * * * * * * * * * *__func__);
>> * * * *}
>> *}
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 8cbbeade..d1ad1c1 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -927,8 +927,7 @@ void omap3_pm_off_mode_enable(int enable)
>> * * * * * * * * * * * * * * * *pwrst->pwrdm == core_pwrdm&&
>> * * * * * * * * * * * * * * * *state == PWRDM_POWER_OFF) {
>> * * * * * * * * * * * *pwrst->next_state = PWRDM_POWER_RET;
>> - * * * * * * * * * * * WARN_ONCE(1,
>> - * * * * * * * * * * * * * * * "%s: Core OFF disabled due to errata
>> i583
",
>> + * * * * * * * * * * * pr_warning("%s: Core OFF disabled due to errata
>> i583
",
>> * * * * * * * * * * * * * * * *__func__);
>> * * * * * * * *} else {
>> * * * * * * * * * * * *pwrst->next_state = state;
>
> Is this really necessary ? While the WARN_ONCE() is a bit more verbose, it
> has no deleterious impact. We'd rather not carry any more delta from
> upstream then we have to.
>

I agree here, how about send it out to upstream if it is very annoy to
our end users?
We can easy to cherry-pick it back then.

Thanks,
--
Bryan Wu <bryan.wu@canonical.com>
Kernel Developer * *+86.138-1617-6545 Mobile
Ubuntu Kernel Team
Canonical Ltd. * * *www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Ricardo Salveti 01-30-2011 11:26 AM

OMAP3630: PM: don't warn the user with a trace in case of PM34XX_ERRATUM
 
On Sun, Jan 30, 2011 at 10:04 AM, Bryan Wu <bryan.wu@canonical.com> wrote:
> On Sat, Jan 29, 2011 at 4:42 AM, Tim Gardner <tim.gardner@canonical.com> wrote:
>> On 01/28/2011 10:17 AM, Ricardo Salveti de Araujo wrote:
>>>
>>> In case in user has a OMAP3630< *ES1.2 the kernel should warn the user
>>> about the ERRATUM, but using pr_warning instead of WARN_ON is already
>>> enough, as there is nothing else the user can do besides changing the
>>> board.
>>>
>>> With this we avoid having the following calltrace while booting with some
>>> Beagle xM revisions:
>>> WARNING: at
>>> /home/apw/build/ubuntu-natty2/ubuntu-natty2/arch/arm/mach-omap2/cpuidle34xx.c:468
>>> omap_init_power_states+0x230/0x238()
>>> omap_init_power_states: core off state C7 disabled due to i583
>>> Modules linked in:
>>> [<c00596f4>] (unwind_backtrace+0x0/0xfc) from [<c04f29a8>]
>>> (dump_stack+0x18/0x1c)
>>> [<c04f29a8>] (dump_stack+0x18/0x1c) from [<c008510c>]
>>> (warn_slowpath_common+0x5c/0x6c)
>>> [<c008510c>] (warn_slowpath_common+0x5c/0x6c) from [<c00851c0>]
>>> (warn_slowpath_fmt+0x38/0x40)
>>> [<c00851c0>] (warn_slowpath_fmt+0x38/0x40) from [<c00676f4>]
>>> (omap_init_power_states+0x230/0x238)
>>> [<c00676f4>] (omap_init_power_states+0x230/0x238) from [<c00131a0>]
>>> (omap3_idle_init+0x74/0x18c)
>>> [<c00131a0>] (omap3_idle_init+0x74/0x18c) from [<c00126b4>]
>>> (omap3_pm_init+0x1ac/0x308)
>>> [<c00126b4>] (omap3_pm_init+0x1ac/0x308) from [<c00474c0>]
>>> (do_one_initcall+0x3c/0x1b4)
>>> [<c00474c0>] (do_one_initcall+0x3c/0x1b4) from [<c0008d58>]
>>> (kernel_init+0xe0/0x178)
>>> [<c0008d58>] (kernel_init+0xe0/0x178) from [<c00532c8>]
>>> (kernel_thread_exit+0x0/0x8)
>>> ---[ end trace e639b107cbbc60f1 ]---
>>>
>>> Signed-off-by: Ricardo Salveti de Araujo<ricardo.salveti@canonical.com>
>>> ---
>>> *arch/arm/mach-omap2/cpuidle34xx.c | * *2 +-
>>> *arch/arm/mach-omap2/pm34xx.c * * *| * *3 +--
>>> *2 files changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
>>> b/arch/arm/mach-omap2/cpuidle34xx.c
>>> index f7b22a1..54ec28e 100644
>>> --- a/arch/arm/mach-omap2/cpuidle34xx.c
>>> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
>>> @@ -464,7 +464,7 @@ void omap_init_power_states(void)
>>> * * * *if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {
>>> * * * * * * * *omap3_power_states[OMAP3_STATE_C7].valid = 0;
>>> * * * * * * * *cpuidle_params_table[OMAP3_STATE_C7].valid = 0;
>>> - * * * * * * * WARN_ONCE(1, "%s: core off state C7 disabled due to
>>> i583
",
>>> + * * * * * * * pr_warning("%s: core off state C7 disabled due to i583
",
>>> * * * * * * * * * * * * * * * *__func__);
>>> * * * *}
>>> *}
>>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>>> index 8cbbeade..d1ad1c1 100644
>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>> @@ -927,8 +927,7 @@ void omap3_pm_off_mode_enable(int enable)
>>> * * * * * * * * * * * * * * * *pwrst->pwrdm == core_pwrdm&&
>>> * * * * * * * * * * * * * * * *state == PWRDM_POWER_OFF) {
>>> * * * * * * * * * * * *pwrst->next_state = PWRDM_POWER_RET;
>>> - * * * * * * * * * * * WARN_ONCE(1,
>>> - * * * * * * * * * * * * * * * "%s: Core OFF disabled due to errata
>>> i583
",
>>> + * * * * * * * * * * * pr_warning("%s: Core OFF disabled due to errata
>>> i583
",
>>> * * * * * * * * * * * * * * * *__func__);
>>> * * * * * * * *} else {
>>> * * * * * * * * * * * *pwrst->next_state = state;
>>
>> Is this really necessary ? While the WARN_ONCE() is a bit more verbose, it
>> has no deleterious impact. We'd rather not carry any more delta from
>> upstream then we have to.
>>
>
> I agree here, how about send it out to upstream if it is very annoy to
> our end users?
> We can easy to cherry-pick it back then.

Already did, just need to do a minor change and it'll probably be accepted.

Cheers,
--
Ricardo Salveti de Araujo

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

Ricardo Salveti 02-01-2011 02:24 AM

OMAP3630: PM: don't warn the user with a trace in case of PM34XX_ERRATUM
 
On Sun, Jan 30, 2011 at 2:07 PM, Andy Whitcroft <apw@canonical.com> wrote:
> On Fri, Jan 28, 2011 at 01:42:25PM -0700, Tim Gardner wrote:
>
>> Is this really necessary ? While the WARN_ONCE() is a bit more
>> verbose, it has no deleterious impact. We'd rather not carry any
>> more delta from upstream then we have to.
>
> I am inclined to suggest this is a good thing as we will be triggering
> apport for something which essentially is unfixable. *I suspect that as
> this will also trigger kerneloops that upstream will take the change.
> Indeed, it sounds like it will go upstream from the rest of the thread.

Patch queued at Kevin Hilman's for_2.6.39/pm-misc branch, so it should
be upstream soon, but not for 38.

Cheers,
--
Ricardo Salveti de Araujo

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team


All times are GMT. The time now is 11:27 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.