Vitaly Wool <vitalywool@xxxxxxxxx> writes:
> On Wed, Dec 8, 2010 at 11:40 PM, Kevin Hilman
> <khilman@xxxxxxxxxxxxxxxxxxx> wrote:
>> @@ -120,8 +133,9 @@ static void omap2_enter_full_retention(void)
>> Â Â Â Â Â Â Â Âgoto no_sleep;
>>
>> Â Â Â Â/* Block console output in case it is on one of the OMAP UARTs */
>> - Â Â Â if (try_acquire_console_sem())
>> - Â Â Â Â Â Â Â goto no_sleep;
>> + Â Â Â if (!is_suspending())
>> + Â Â Â Â Â Â Â if (try_acquire_console_sem())
>> + Â Â Â Â Â Â Â Â Â Â Â goto no_sleep;
>
> Combine into one if?
personal preference I guess. I prefer it nested.
>
> <snip>
>
>> Â Â Â Âomap_uart_prepare_idle(0);
>> Â Â Â Âomap_uart_prepare_idle(1);
>> @@ -136,7 +150,8 @@ static void omap2_enter_full_retention(void)
>> Â Â Â Âomap_uart_resume_idle(1);
>> Â Â Â Âomap_uart_resume_idle(0);
>>
>> - Â Â Â release_console_sem();
>> + Â Â Â if (!is_suspending())
>> + Â Â Â Â Â Â Â release_console_sem();
>>
>> Âno_sleep:
>> Â Â Â Âif (omap2_pm_debug) {
>> @@ -284,6 +299,12 @@ out:
>> Â Â Â Âlocal_irq_enable();
>> Â}
>>
>> +static int omap2_pm_begin(suspend_state_t state)
>> +{
>> + Â Â Â suspend_state = state;
>> + Â Â Â return 0;
>> +}
>
> Do you really need a return code here?
yes, this function prototype is defined by the driver model, not by me.
>> Âstatic int omap2_pm_prepare(void)
>> Â{
>> Â Â Â Â/* We cannot sleep in idle until we have resumed */
>> @@ -333,10 +354,18 @@ static void omap2_pm_finish(void)
>> Â Â Â Âenable_hlt();
>> Â}
>>
>> +static void omap2_pm_end(void)
>> +{
>> + Â Â Â suspend_state = PM_SUSPEND_ON;
>> + Â Â Â return;
>> +}
>
> Redundant return.
but harmless
>> +
>> Âstatic struct platform_suspend_ops omap_pm_ops = {
>> +    .begin     Â= omap2_pm_begin,
>>    Â.prepare    Â= omap2_pm_prepare,
>>    Â.enter     Â= omap2_pm_enter,
>>    Â.finish     = omap2_pm_finish,
>> +    .end      Â= omap2_pm_end,
>>    Â.valid     Â= suspend_valid_only_mem,
>> Â};
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 0ec8a04..648b8c5 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -50,6 +50,19 @@
>> Â#include "sdrc.h"
>> Â#include "control.h"
>>
>> +#ifdef CONFIG_SUSPEND
>> +static suspend_state_t suspend_state = PM_SUSPEND_ON;
>> +static inline bool is_suspending(void)
>> +{
>> + Â Â Â return (suspend_state != PM_SUSPEND_ON);
>> +}
>> +#else
>> +static inline bool is_suspending(void)
>> +{
>> + Â Â Â return false;
>> +}
>> +#endif
>> +
>> Â/* Scratchpad offsets */
>> Â#define OMAP343X_TABLE_ADDRESS_OFFSET Â Â 0xc4
>> Â#define OMAP343X_TABLE_VALUE_OFFSET Â Â Â 0xc0
>> @@ -387,10 +400,11 @@ void omap_sram_idle(void)
>> Â Â Â Â}
>>
>> Â Â Â Â/* Block console output in case it is on one of the OMAP UARTs */
>> - Â Â Â if (per_next_state < PWRDM_POWER_ON ||
>> - Â Â Â Â Â core_next_state < PWRDM_POWER_ON)
>> - Â Â Â Â Â Â Â if (try_acquire_console_sem())
>> - Â Â Â Â Â Â Â Â Â Â Â goto console_still_active;
>> + Â Â Â if (!is_suspending())
>> + Â Â Â Â Â Â Â if (per_next_state < PWRDM_POWER_ON ||
>> + Â Â Â Â Â Â Â Â Â core_next_state < PWRDM_POWER_ON)
>> + Â Â Â Â Â Â Â Â Â Â Â if (try_acquire_console_sem())
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto console_still_active;
>
> Oh well, 3 nested ifs...
again, personal preference.
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
