[Natty] [PATCH] OMAP3630: PM: don't warn the user with a trace in case of PM34XX_ERRATUM

[Bryan Wu]

On Sat, Jan 29, 2011 at 4:42 AM, Tim Gardner <tim.gardner at 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 at 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\n",
>> +               pr_warning("%s: core off state C7 disabled due to i583\n",
>>                                __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\n",
>> +                       pr_warning("%s: Core OFF disabled due to errata
>> i583\n",
>>                                __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 at 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