Russell King - ARM Linux <li...@arm.linux.org.uk> writes:

> On Fri, May 15, 2009 at 11:40:41AM -0700, Kevin Hilman wrote:
>> This patch is to sync the core linux-omap PM code with mainline.  This
>> code has evolved and been used for a while the linux-omap tree, but
>> the attempt here is to finally get this into mainline.
>> 
>> Following this will be a series of patches from the 'PM branch' of the
>> linux-omap tree to add full PM hardware support from the linux-omap
>> tree.
>> 
>> Much of this PM core code was written by Jouni Hogander with
>> significant contributions from Paul Walmsley as well as many others
>> from Nokia, Texas Instruments and linux-omap community.
>
> Overall comment, I think we need to rework the idle support code so
> that enable_hlt/disable_hlt can be used even when pm_idle has been
> overridden, rather than OMAP going off and inventing its own mechanisms.

Would adding:

        if (hlt_counter)
                cpu_relax();

to the beginning of omap*_pm_idle functions be sufficient?  That will
at least allow the hlt stuff to behave as expected.

The only thing the OMAP /sys/power/sleep_while_idle hook adds to this
functionality is the ability to control this from sysfs.

Any objections to /sys/power/enable_hlt?

>> diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
>> new file mode 100644
>> index 0000000..2a2d1a3
>> --- /dev/null
>> +++ b/arch/arm/mach-omap2/pm24xx.c
>> @@ -0,0 +1,557 @@
>> +/*
>> + * OMAP2 Power Management Routines
>> + *
>> + * Copyright (C) 2005 Texas Instruments, Inc.
>> + * Copyright (C) 2006-2008 Nokia Corporation
>> + *
>> + * Written by:
>> + * Richard Woodruff <r-woodru...@ti.com>
>> + * Tony Lindgren
>> + * Juha Yrjola
>> + * Amit Kucheria <amit.kuche...@nokia.com>
>> + * Igor Stoppa <igor.sto...@nokia.com>
>> + *
>> + * Based on pm.c for omap1
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/suspend.h>
>> +#include <linux/sched.h>
>> +#include <linux/proc_fs.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/irq.h>
>> +#include <linux/time.h>
>> +
>> +#include <asm/mach/time.h>
>> +#include <asm/mach/irq.h>
>> +#include <asm/mach-types.h>
>> +
>> +#include <mach/irqs.h>
>> +#include <mach/clock.h>
>> +#include <mach/sram.h>
>> +#include <mach/control.h>
>> +#include <mach/gpio.h>
>
> Should be linux/gpio.h

Ack

>> +/*
>> + * Note that you can use clock_event_device->min_delta_ns if you want to
>> + * avoid reprogramming timer too often when using CONFIG_NO_HZ.
>> + */
>> +static void omap2_pm_idle(void)
>> +{
>> +    local_irq_disable();
>> +    local_fiq_disable();
>> +
>> +    if (!omap2_can_sleep()) {
>> +            if (!atomic_read(&sleep_block) && omap2_irq_pending())
>> +                    goto out;
>> +            omap2_enter_mpu_retention();
>> +            goto out;
>> +    }
>> +
>> +    if (omap2_irq_pending())
>> +            goto out;
>> +
>> +    omap2_enter_full_retention();
>> +
>> +out:
>> +    local_fiq_enable();
>> +    local_irq_enable();
>> +}
>
> It's totally unclear what the comment above the function has to do with
> the function itself.

Indeed.  Will be removed.

>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> new file mode 100644
>> index 0000000..0fb6bec
>> --- /dev/null
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -0,0 +1,607 @@
>> +/*
>> + * OMAP3 Power Management Routines
>> + *
>> + * Copyright (C) 2006-2008 Nokia Corporation
>> + * Tony Lindgren <t...@atomide.com>
>> + * Jouni Hogander
>> + *
>> + * Copyright (C) 2005 Texas Instruments, Inc.
>> + * Richard Woodruff <r-woodru...@ti.com>
>> + *
>> + * Based on pm.c for omap1
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/pm.h>
>> +#include <linux/suspend.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/module.h>
>> +#include <linux/list.h>
>> +#include <linux/err.h>
>> +
>> +#include <mach/gpio.h>
>
> Should be linux/gpio.h

Ack

>> +static void omap3_pm_idle(void)
>> +{
>> +    local_irq_disable();
>> +    local_fiq_disable();
>> +
>> +    if (!omap3_can_sleep())
>> +            goto out;
>> +
>> +    if (omap_irq_pending())
>> +            goto out;
>
> So what happens if an IRQ becomes pending at this precise point?

Then it gets missed this time, and will be triggered upon wakeup.  If
it's a wakeup-capable interrupt, then it will wake the system
immediately.

In subsequent series of the PM branch, this will be going away in
favor of using the [enable|disable]_irq_wake() and the lazy IRQ
disabling features.

>> +
>> +    omap_sram_idle();
>> +
>> +out:
>> +    local_fiq_enable();
>> +    local_irq_enable();
>> +}
>> +    /* IRQ mode */
>> +    bic    r0, r7, #0x1F
>> +    orr    r0, r0, #0x12
>> +    msr    cpsr, r0 /*go into IRQ mode*/
>> +    ldmia  r3!,{r4-r6}      /*load the SP and LR from SDRAM*/
>> +    mov    sp, r4   /*update the SP */
>> +    mov    lr, r5   /*update the LR */
>> +    msr    spsr, r6 /*update the SPSR */
>> +
>> +    /* ABORT mode */
>> +    bic    r0, r7, #0x1F
>> +    orr    r0, r0, #0x17
>> +    msr    cpsr, r0 /* go into ABORT mode */
>> +    ldmia  r3!,{r4-r6}      /*load the SP and LR from SDRAM */
>> +    mov    sp, r4           /*update the SP */
>> +    mov    lr, r5           /*update the LR */
>> +    msr    spsr, r6         /*update the SPSR */
>> +
>> +    /* UNDEEF mode */
>> +    bic    r0, r7, #0x1F
>> +    orr    r0, r0, #0x1B
>> +    msr    cpsr, r0         /*go into UNDEF mode */
>> +    ldmia  r3!,{r4-r6}      /*load the SP and LR from SDRAM */
>> +    mov    sp, r4           /*update the SP*/
>> +    mov    lr, r5           /*update the LR*/
>> +    msr    spsr, r6         /*update the SPSR*/
>> +
>> +    /* SYSTEM (USER) mode */
>> +    bic    r0, r7, #0x1F
>> +    orr    r0, r0, #0x1F
>> +    msr    cpsr, r0         /*go into USR mode */
>> +    ldmia  r3!,{r4-r6}      /*load the SP and LR from SDRAM*/
>> +    mov    sp, r4           /*update the SP */
>> +    mov    lr, r5           /*update the LR */
>> +    msr    spsr, r6         /*update the SPSR */
>> +    msr    cpsr, r7         /*back to original mode*/
>
> There is a function which re-initializes the abort mode registers already -
> cpu_init().  Please use that if possible instead.

OK, will investigate.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to