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

[Ricardo Salveti]

On Sun, Jan 30, 2011 at 10:04 AM, Bryan Wu <bryan.wu at canonical.com> wrote:
> 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.

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

Cheers,
-- 
Ricardo Salveti de Araujo