Implement arch disable/enable irq hooks.
Subsystems (2)
PATCH 1/7 Implement arch disable/enable irq hooks.
These hooks ensure that a decrementer interrupt is not pending when suspending; otherwise, problems may occur. For example, with deep sleep on the 831x, a pending decrementer will cause a system freeze because the SoC thinks the decrementer interrupt would have woken the system, but the core must have interrupts disabled due to the setup required for deep sleep.
---
Please queue these patches for 2.6.25 if they do not offend.
arch/powerpc/kernel/time.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/asm-powerpc/machdep.h | 13 +++++++++++++ 2 files changed, 54 insertions(+), 0 deletions(-)
arch/powerpc/kernel/time.c +41 -0
@@ -647,6 +647,47 @@ void wakeup_decrementer(void)set_dec (ticks); } +#ifdef CONFIG_SUSPEND +void generic_suspend_disable_irqs (void ) +{ +preempt_disable (); + +/* Disable the decrementer, so that it doesn't interfere +* with suspending. +*/ + +set_dec (0x7fffffff ); +hard_irq_disable (); +set_dec (0x7fffffff ); +} + +void generic_suspend_enable_irqs (void ) +{ +wakeup_decrementer (); + +local_irq_enable (); +preempt_enable (); +} + +/* Overrides the weak version in kernel/power/main.c */ +void arch_suspend_disable_irqs (void ) +{ +if (ppc_md.suspend_disable_irqs) + ppc_md.suspend_disable_irqs (); +else +generic_suspend_disable_irqs (); +} + +/* Overrides the weak version in kernel/power/main.c */ +void arch_suspend_enable_irqs (void ) +{ +if (ppc_md.suspend_enable_irqs) + ppc_md.suspend_enable_irqs (); +else +generic_suspend_enable_irqs (); +} +#endif +#ifdef CONFIG_SMP void __init smp_space_timers (unsigned int max_cpus) {
include/asm-powerpc/machdep.h +13 -0
@@ -253,6 +253,16 @@ struct machdep_calls {*/ void (*machine_kexec)(struct kimage *image);#endif /* CONFIG_KEXEC */ + +#ifdef CONFIG_SUSPEND +/* These are called to disable and enable, respectively, IRQs when +* entering a suspend state. If NULL, then the generic versions +* will be called. The generic versions disable/enable the +* decrementer along with interrupts. +*/ +void (*suspend_disable_irqs)(void ); +void (*suspend_enable_irqs)(void ); +#endif };extern void power4_idle (void );
@@ -326,5 +336,8 @@ static inline void log_error(char *buf, unsigned int err_type, int fatal) ppc_md.log_error (buf, err_type, fatal); } +void generic_suspend_disable_irqs (void ); +void generic_suspend_enable_irqs (void ); +#endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_MACHDEP_H */
Signature · 1 line
1.5.3.4
PATCH 2/7 pm: Add TLF_SLEEPING hack to delay interrupt delivery when waking from sleep.
The e300 core (and probably most other 6xx chips) can only come out of sleep mode with an interrupt. However, interrupts are logically disabled by the power management layer.
This hack extends the existing doze/nap hack to also suppress the running of the interrupt handler when in sleep mode.
--- arch/powerpc/kernel/entry_32.S | 6 ++-- arch/powerpc/kernel/idle_6xx.S | 41 ++++++++++++++++++++++++++++++++++++- include/asm-powerpc/thread_info.h | 2 + 3 files changed, 45 insertions(+), 4 deletions(-)
arch/powerpc/kernel/entry_32.S +3 -3
@@ -138,8 +138,8 @@ transfer_to_handler: #ifdef CONFIG_6xx tophys(r9,r9) /* check local flags */ lwz r12,TI_LOCAL_FLAGS(r9) - mtcrf 0x01,r12 - bt- 31-TLF_NAPPING,4f + andi. r8, r12, _TLF_NAPPING | _TLF_SLEEPING + bne- 4f #endif /* CONFIG_6xx */ .globl transfer_to_handler_cont transfer_to_handler_cont:
@@ -154,7 +154,7 @@ transfer_to_handler_cont: RFI /* jump to handler, enable MMU */ #ifdef CONFIG_6xx -4: rlwinm r12,r12,0,~_TLF_NAPPING +4: rlwinm r12,r12,0,~(_TLF_NAPPING | _TLF_SLEEPING) stw r12,TI_LOCAL_FLAGS(r9) b power_save_6xx_restore #endif
arch/powerpc/kernel/idle_6xx.S +40 -1
62 lines not on this page · raw message
include/asm-powerpc/thread_info.h +2 -0
@@ -151,8 +151,10 @@ static inline struct thread_info *current_thread_info(void)/* Bits in local_flags */ /* Don't move TLF_NAPPING without adjusting the code in entry_32.S */ #define TLF_NAPPING 0/* idle thread enabled NAP mode */ +#define TLF_SLEEPING 1/* suspend code enabled SLEEP mode */ #define _TLF_NAPPING (1 << TLF_NAPPING) +#define _TLF_SLEEPING (1 << TLF_SLEEPING)#endif /* __KERNEL__ */
Signature · 1 line
1.5.3.4
PATCH 3/7 Add 6xx-style HID0_SLEEP support.
In reply to Scott Wood--- arch/powerpc/sysdev/6xx-suspend.S | 52 +++++++++++++++++++++++++++++++++++++ arch/powerpc/sysdev/Makefile | 4 +++ include/asm-powerpc/mpc6xx.h | 6 ++++ 3 files changed, 62 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/sysdev/6xx-suspend.S create mode 100644 include/asm-powerpc/mpc6xx.h
arch/powerpc/sysdev/6xx-suspend.S new file +52 -0
@@ -0,0 +1,52 @@ +/* + * Enter and leave sleep state on chips with 6xx-style HID0 + * power management bits, which don't leave sleep state via reset. + * + * Author: Scott Wood <[email protected]> + * + * Copyright (c) 2006-2007 Freescale Semiconductor, Inc. + * + * 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 <asm/ppc_asm.h> +#include <asm/reg.h> +#include <asm/thread_info.h> +#include <asm/asm-offsets.h> + +_GLOBAL(mpc6xx_enter_standby) + mflr r4 + + mfspr r5, SPRN_HID0 + rlwinm r5, r5, 0, ~(HID0_DOZE | HID0_NAP) + oris r5, r5, HID0_SLEEP@h + mtspr SPRN_HID0, r5 + isync + + lis r5, ret_from_standby@h + ori r5, r5, ret_from_standby@l + mtlr r5 + + rlwinm r5, r1, 0, 0, 31-THREAD_SHIFT + lwz r6, TI_LOCAL_FLAGS(r5) + ori r6, r6, _TLF_SLEEPING + stw r6, TI_LOCAL_FLAGS(r5) + + mfmsr r5 + ori r5, r5, MSR_EE + oris r5, r5, MSR_POW@h + sync + mtmsr r5 + isync + +1: b 1b + +ret_from_standby: + mfspr r5, SPRN_HID0 + rlwinm r5, r5, 0, ~HID0_SLEEP + mtspr SPRN_HID0, r5 + + mtlr r4 + blr
arch/powerpc/sysdev/Makefile +4 -0
@@ -37,3 +37,7 @@ obj-$(CONFIG_PPC_DCR) += dcr.o obj-$(CONFIG_8xx) += mpc8xx_pic.o commproc.o obj-$(CONFIG_UCODE_PATCH) += micropatch.o endif + +ifeq ($(CONFIG_SUSPEND),y) +obj-$(CONFIG_6xx) += 6xx-suspend.o +endif
include/asm-powerpc/mpc6xx.h new file +6 -0
@@ -0,0 +1,6 @@ +#ifndef __ASM_POWERPC_MPC6xx_H +#define __ASM_POWERPC_MPC6xx_H + +void mpc6xx_enter_standby (void ); + +#endif
Signature · 1 line
1.5.3.4
PATCH 4/7 fsl_soc: Factor fsl_get_sys_freq() out of the wdt init.
In reply to Scott Wood--- arch/powerpc/sysdev/fsl_soc.c | 44 ++++++++++++++++++++++------------------ arch/powerpc/sysdev/fsl_soc.h | 1 + 2 files changed, 25 insertions(+), 20 deletions(-)
arch/powerpc/sysdev/fsl_soc.c +24 -20
@@ -66,6 +66,27 @@ phys_addr_t get_immrbase(void)EXPORT_SYMBOL (get_immrbase); +u32 fsl_get_sys_freq (void ) +{ +struct device_node *soc =of_find_node_by_type (NULL ,"soc" ); +const u32 *freq; +u32 ret =0 ; + +if (!soc) + goto err; + + freq =of_get_property (soc,"bus-frequency" ,NULL ); +if (!freq) + goto err; + + ret = *freq; + +err: +of_node_put (soc); +return ret; +} +EXPORT_SYMBOL (fsl_get_sys_freq); +#if defined(CONFIG_CPM2 ) || defined(CONFIG_8xx)static u32 brgfreq =-1 ;
@@ -450,9 +471,9 @@ arch_initcall(fsl_i2c_of_init);static int __init mpc83xx_wdt_init (void ) {struct resource r; -struct device_node *soc, *np; +struct device_node *np;struct platform_device *dev; -const unsigned int *freq; +u32 freq =fsl_get_sys_freq ();int ret; np =of_find_compatible_node (NULL ,"watchdog" ,"mpc83xx_wdt" );
@@ -462,19 +483,6 @@ static int __init mpc83xx_wdt_init(void) goto nodev; } - soc =of_find_node_by_type (NULL ,"soc" ); - -if (!soc) { - ret = -ENODEV ; - goto nosoc; - } - - freq =of_get_property (soc,"bus-frequency" ,NULL ); -if (!freq) { - ret = -ENODEV ; - goto err; - } -memset (&r,0 ,sizeof (r)); ret =of_address_to_resource (np,0 , &r);
@@ -487,20 +495,16 @@ static int __init mpc83xx_wdt_init(void) goto err; } - ret =platform_device_add_data (dev, freq,sizeof (int )); + ret =platform_device_add_data (dev, &freq,sizeof (int ));if (ret) goto unreg; -of_node_put (soc);of_node_put (np); -return 0 ; unreg:platform_device_unregister (dev); err: -of_node_put (soc); -nosoc:of_node_put (np); nodev:return ret;
arch/powerpc/sysdev/fsl_soc.h +1 -0
@@ -7,6 +7,7 @@extern phys_addr_t get_immrbase (void );extern u32 get_brgfreq (void );extern u32 get_baudrate (void ); +extern u32 fsl_get_sys_freq (void );struct spi_board_info ;
Signature · 1 line
1.5.3.4
PATCH 5/7 mpc83xx: Power Management support
In reply to Scott WoodBasic PM support for 83xx. Standby is implemented as sleep. Suspend-to-RAM is implemented as "deep sleep" (with the processor turned off) on 831x.
--- arch/powerpc/platforms/83xx/Kconfig | 5 + arch/powerpc/platforms/83xx/Makefile | 1 + arch/powerpc/platforms/83xx/suspend-asm.S | 531 +++++++++++++++++++++++++++++ arch/powerpc/platforms/83xx/suspend.c | 423 +++++++++++++++++++++++ arch/powerpc/sysdev/fsl_soc.c | 33 ++ arch/powerpc/sysdev/fsl_soc.h | 10 + arch/powerpc/sysdev/ipic.c | 71 ++++ include/asm-powerpc/reg.h | 4 + include/linux/fsl_devices.h | 24 ++ 9 files changed, 1102 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/platforms/83xx/suspend-asm.S create mode 100644 arch/powerpc/platforms/83xx/suspend.c
arch/powerpc/platforms/83xx/Kconfig +5 -0
@@ -75,3 +75,8 @@ config PPC_MPC836x select PPC_UDBG_16550 select PPC_INDIRECT_PCI default y if MPC836x_MDS + +config PPC_83xx_SUSPEND + bool + default y + depends on PPC_83xx && SUSPEND
arch/powerpc/platforms/83xx/Makefile +1 -0
@@ -3,6 +3,7 @@ # obj-y := misc.o usb.o obj-$(CONFIG_PCI) += pci.o +obj-$(CONFIG_SUSPEND) += suspend.o suspend-asm.o obj-$(CONFIG_MPC8313_RDB) += mpc8313_rdb.o obj-$(CONFIG_MPC832x_RDB) += mpc832x_rdb.o obj-$(CONFIG_MPC834x_MDS) += mpc834x_mds.o
arch/powerpc/platforms/83xx/suspend-asm.S new file +531 -0
531 lines not on this page · raw message
arch/powerpc/platforms/83xx/suspend.c new file +423 -0
423 lines not on this page · raw message
arch/powerpc/sysdev/fsl_soc.c +33 -0
@@ -151,6 +151,33 @@ u32 get_baudrate(void)EXPORT_SYMBOL (get_baudrate);#endif /* CONFIG_CPM2 */ +int fsl_sleep_init (struct fsl_sleep_platform_data *sleep, +struct device_node *node) +{ +int proplen, ret = -ENODEV ; +const u32 *sleepdata =of_get_property (node,"sleep" , &proplen); +struct device_node *sleep_controller; + +if (!sleepdata || proplen !=8 ) +return -ENODEV ; + + sleep_controller =of_find_node_by_phandle (sleepdata[0 ]); +if (!sleep_controller) +return -ENODEV ; + +/* There can only be one fsl,mpc83xx-pmc device in the system; +* it is assumed that it is the one that the pmc driver matches. +*/ +if (of_device_is_compatible (sleep_controller,"fsl,mpc83xx-pmc" )) { + sleep->sccr_mask = sleepdata[1 ]; + ret =0 ; + } + +of_node_put (sleep_controller); +return ret; +} +EXPORT_SYMBOL (fsl_sleep_init); +static int __init gfar_mdio_of_init (void ) {struct device_node *np;
@@ -321,6 +348,8 @@ static int __init gfar_of_init(void)of_node_put (phy);of_node_put (mdio); +fsl_sleep_init (&gfar_data.sleep, np); + ret = platform_device_add_data(gfar_dev, &gfar_data, sizeof(struct
@@ -576,6 +605,8 @@ static int __init fsl_usb_of_init(void) prop =of_get_property (np,"phy_type" ,NULL ); usb_data.phy_mode =determine_usb_phy (prop); +fsl_sleep_init (&usb_data.sleep, np); + ret = platform_device_add_data(usb_dev_mph, &usb_data, sizeof(struct
@@ -640,6 +671,8 @@ static int __init fsl_usb_of_init(void) prop =of_get_property (np,"phy_type" ,NULL ); usb_data.phy_mode =determine_usb_phy (prop); +fsl_sleep_init (&usb_data.sleep, np); +if (usb_dev_dr_host) { usb_dev_dr_host->dev.coherent_dma_mask =0xffffffffUL ; usb_dev_dr_host->dev.dma_mask = &usb_dev_dr_host->
arch/powerpc/sysdev/fsl_soc.h +10 -0
@@ -10,12 +10,22 @@ extern u32 get_baudrate(void);extern u32 fsl_get_sys_freq (void );struct spi_board_info ; +struct fsl_sleep_platform_data ; +struct device_node ;extern int fsl_spi_init (struct spi_board_info *board_infos,unsigned int num_board_infos,void (*activate_cs)(u8 cs,u8 polarity),void (*deactivate_cs)(u8 cs,u8 polarity)); +int fsl_sleep_init (struct fsl_sleep_platform_data *sleep, +struct device_node *node); + +/* Calls to fsl_sleep_dev and fsl_wake_dev cannot be nested. */ +void fsl_sleep_dev (struct fsl_sleep_platform_data *sleep); +void fsl_wake_dev (struct fsl_sleep_platform_data *sleep); +extern void fsl_rstcr_restart (char *cmd); +#endif #endif
arch/powerpc/sysdev/ipic.c +71 -0
85 lines not on this page · raw message
include/asm-powerpc/reg.h +4 -0
@@ -149,7 +149,9 @@#define CTRL_RUNLATCH 0x1#define SPRN_DABR 0x3F5/* Data Address Breakpoint Register */ #define DABR_TRANSLATION (1UL << 2) +#define SPRN_DABR2 0x13D/* 83xx */ #define SPRN_DAR 0x013/* Data Address Register */ +#define SPRN_DBCR 0x136/* 83xx Data Breakpoint Control Reg */ #define SPRN_DSISR 0x012/* Data Storage Interrupt Status Register */ #define DSISR_NOHPTE 0x40000000/* no translation found */ #define DSISR_PROTFAULT 0x08000000/* protection fault */
@@ -255,6 +257,8 @@#define HID1_PS (1<<16)/* 750FX PLL selection */ #define SPRN_HID2 0x3F8/* Hardware Implementation Register 2 */ #define SPRN_IABR 0x3F2/* Instruction Address Breakpoint Register */ +#define SPRN_IABR2 0x3FA/* 83xx */ +#define SPRN_IBCR 0x135/* 83xx Insn Breakpoint Control Reg */ #define SPRN_HID4 0x3F4/* 970 HID4 */ #define SPRN_HID5 0x3F6/* 970 HID5 */ #define SPRN_HID6 0x3F9/* BE HID 6 */
include/linux/fsl_devices.h +24 -0
@@ -45,10 +45,27 @@* */ +struct fsl_sleep_platform_data { +/* +* The bits set in this mask will be cleared in the SCCR +* when put to sleep, and restored on wakeup. +*/ +u32 sccr_mask; +}; + +/* +* Calls to fsl_sleep_device do not nest -- if you call +* sleep twice without an intervening wake, you will not +* be able to wake the device again. +*/ +void fsl_sleep_device (struct fsl_sleep_platform_data *data); +void fsl_wake_device (struct fsl_sleep_platform_data *data); +struct gianfar_platform_data {/* device specific information */ u32 device_flags;/* board specific information */ +struct fsl_sleep_platform_data sleep;u32 board_flags;u32 bus_id;u32 phy_id;
@@ -104,6 +121,7 @@ struct fsl_usb2_platform_data {enum fsl_usb2_operating_modes operating_mode;enum fsl_usb2_phy_modes phy_mode;unsigned int port_enables; +struct fsl_sleep_platform_data sleep; };/* Flags in fsl_usb2_mph_platform_data */
@@ -126,5 +144,11 @@ struct mpc8xx_pcmcia_ops {int (*voltage_set)(int slot,int vcc,int vpp); }; +/* Returns non-zero if the current suspend operation would +* lead to a deep sleep (i.e. power removed from the core, +* instead of just the clock). +*/ +int fsl_deep_sleep (void ); +#endif /* _FSL_DEVICE_H_ */ #endif /* __KERNEL__ */
Signature · 1 line
1.5.3.4
PATCH 6/7 mpc83xx: timer driver for PM wakeup
In reply to Scott WoodThis is a driver for the mpc83xx's GTM4 timer. It's functionality is limited to providing a wakeup source for suspend-to-RAM.
--- arch/powerpc/platforms/83xx/Kconfig | 9 + arch/powerpc/platforms/83xx/Makefile | 1 + arch/powerpc/platforms/83xx/timer.c | 299 ++++++++++++++++++++++++++++++++++ 3 files changed, 309 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/platforms/83xx/timer.c
arch/powerpc/platforms/83xx/Kconfig +9 -0
@@ -80,3 +80,12 @@ config PPC_83xx_SUSPEND bool default y depends on PPC_83xx && SUSPEND + +config MPC83xx_GTM + tristate "83xx General-Purpose Timers for PM wakeup" + help + This enables a driver for the GTM4 timer, to be used + as a wakeup source for suspend-to-RAM. To arm the + timer, write the number of seconds until expiration + to the "timeout" file in the device's sysfs directory. + To disarm, write 0 to the "timeout" file.
arch/powerpc/platforms/83xx/Makefile +1 -0
@@ -4,6 +4,7 @@ obj-y := misc.o usb.o obj-$(CONFIG_PCI) += pci.o obj-$(CONFIG_SUSPEND) += suspend.o suspend-asm.o +obj-$(CONFIG_MPC83xx_GTM) += timer.o obj-$(CONFIG_MPC8313_RDB) += mpc8313_rdb.o obj-$(CONFIG_MPC832x_RDB) += mpc832x_rdb.o obj-$(CONFIG_MPC834x_MDS) += mpc834x_mds.o
arch/powerpc/platforms/83xx/timer.c new file +299 -0
299 lines not on this page · raw message
Signature · 1 line
1.5.3.4
Re: [PATCH 6/7] mpc83xx: timer driver for PM wakeup
Hi Scott,
Quoted 4 lines · Scott Wood
This is a driver for the mpc83xx's GTM4 timer. It's functionality is limited to providing a wakeup source for suspend-to-RAM.
Signed-off-by Scott Wood <[email protected]>
I think you can use dev_err(&dev->dev
arch/powerpc/platforms/83xx/timer.c line 202
Quoted 11 lines
arch/powerpc/platforms/83xx/timer.c new file +11
-0+static int __devinit gtm_probe (struct of_device *dev, +const struct of_device_id *match) +{ +struct device_node *np = dev->node; +struct resource res; +int ret =0 ; +u32 busfreq =fsl_get_sys_freq (); +struct gtm_priv *priv; + +if (busfreq ==0 ) { +printk (KERN_ERR "gtm: No bus frequency in device tree.\n" );
here
Quoted 8 lines
arch/powerpc/platforms/83xx/timer.c new file +8
-0+return -ENODEV ; + } + + priv =kmalloc (sizeof (struct gtm_priv ),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + +spin_lock_init (&priv->lock);
arch/powerpc/platforms/83xx/timer.c line 220
Quoted 10 lines
arch/powerpc/platforms/83xx/timer.c new file +10
-0+dev_set_drvdata (&dev->dev, priv); + + ret =of_address_to_resource (np,0 , &res); +if (ret) + goto out; + + priv->irq =irq_of_parse_and_map (np,0 ); +if (priv->irq ==NO_IRQ ) { +printk (KERN_ERR "mpc83xx-gtm exists in device tree " +"without an IRQ.\n" );
and here.
Thanks,
Signature · 4 lines
Anton Vorontsov email: [email protected] backup email: [email protected] irc://irc.freenode.net/bd2
PATCH 7/7 gianfar: Add flags for magic packet and MDIO.
In reply to Scott WoodThe magic packet flag indicates that the hardware has this capability. The MDIO flag indicates that this device's registers contain active MDIO registers, and thus this device should not be put to sleep.
--- arch/powerpc/sysdev/fsl_soc.c | 7 +++++++ include/linux/fsl_devices.h | 2 ++ 2 files changed, 9 insertions(+), 0 deletions(-)
arch/powerpc/sysdev/fsl_soc.c +7 -0
@@ -324,6 +324,9 @@ static int __init gfar_of_init(void)else gfar_data.interface =PHY_INTERFACE_MODE_MII ; +if (of_get_property (np,"fsl,magic-packet" ,NULL )) + gfar_data.device_flags |=FSL_GIANFAR_DEV_HAS_MAGIC_PACKET ; + ph =of_get_property (np,"phy-handle" ,NULL ); phy =of_find_node_by_phandle (*ph);
@@ -345,6 +348,10 @@ static int __init gfar_of_init(void) gfar_data.phy_id = *id; gfar_data.bus_id = res.start; +if (res.start >= gfar_dev->resource[0 ].start && + res.start < gfar_dev->resource[0 ].end) + gfar_data.device_flags |=FSL_GIANFAR_DEV_HAS_MDIO ; +of_node_put (phy);of_node_put (mdio);
include/linux/fsl_devices.h +2 -0
@@ -87,6 +87,8 @@ struct gianfar_mdio_data {#define FSL_GIANFAR_DEV_HAS_VLAN 0x00000020#define FSL_GIANFAR_DEV_HAS_EXTENDED_HASH 0x00000040#define FSL_GIANFAR_DEV_HAS_PADDING 0x00000080 +#define FSL_GIANFAR_DEV_HAS_MAGIC_PACKET 0x00000100 +#define FSL_GIANFAR_DEV_HAS_MDIO 0x00000200/* Flags in gianfar_platform_data */ #define FSL_GIANFAR_BRD_HAS_PHY_INTR 0x00000001/* set or use a timer */
Signature · 1 line
1.5.3.4
Quoted 34 lines · On Tue, 23 Oct 2007 16:24:04 -0500 Scott Wood <[email protected]> wrote:
These hooks ensure that a decrementer interrupt is not pending when suspending; otherwise, problems may occur. For example, with deep sleep on the 831x, a pending decrementer will cause a system freeze because the SoC thinks the decrementer interrupt would have woken the system, but the core must have interrupts disabled due to the setup required for deep sleep.
Signed-off-by Scott Wood <[email protected]>---
Please queue these patches for 2.6.25 if they do not offend.arch/powerpc/kernel/time.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/asm-powerpc/machdep.h | 13 +++++++++++++ 2 files changed, 54 insertions(+), 0 deletions(-)arch/powerpc/kernel/time.c +11
-0@@ -647,6 +647,47 @@ void wakeup_decrementer(void)set_dec (ticks); } +#ifdef CONFIG_SUSPEND +void generic_suspend_disable_irqs (void ) +{ +preempt_disable (); + +/* Disable the decrementer, so that it doesn't interfere +* with suspending. +*/ + +set_dec (0x7fffffff ); +hard_irq_disable ();
arch/powerpc/kernel/time.c void wakeup_decrementer(void) · line 670
Quoted 10 lines
arch/powerpc/kernel/time.c +10
-0+set_dec (0x7fffffff ); +} + +void generic_suspend_enable_irqs (void ) +{ +wakeup_decrementer (); + +local_irq_enable (); +preempt_enable (); +}
Should these be static functions? Seems so...
Quoted 45 lines
arch/powerpc/kernel/time.c +20
-0+ +/* Overrides the weak version in kernel/power/main.c */ +void arch_suspend_disable_irqs (void ) +{ +if (ppc_md.suspend_disable_irqs) + ppc_md.suspend_disable_irqs (); +else +generic_suspend_disable_irqs (); +} + +/* Overrides the weak version in kernel/power/main.c */ +void arch_suspend_enable_irqs (void ) +{ +if (ppc_md.suspend_enable_irqs) + ppc_md.suspend_enable_irqs (); +else +generic_suspend_enable_irqs (); +} +#endif +#ifdef CONFIG_SMP void __init smp_space_timers (unsigned int max_cpus) {include/asm-powerpc/machdep.h +10
-0@@ -253,6 +253,16 @@ struct machdep_calls {*/ void (*machine_kexec)(struct kimage *image);#endif /* CONFIG_KEXEC */ + +#ifdef CONFIG_SUSPEND +/* These are called to disable and enable, respectively, IRQs when +* entering a suspend state. If NULL, then the generic versions +* will be called. The generic versions disable/enable the +* decrementer along with interrupts. +*/ +void (*suspend_disable_irqs)(void ); +void (*suspend_enable_irqs)(void ); +#endif };extern void power4_idle (void );
include/asm-powerpc/machdep.h static inline void log_error(char *buf, unsigned int err_type, int fatal) · line 340
Quoted 5 lines
include/asm-powerpc/machdep.h +2
-0@@ -326,5 +336,8 @@ static inline void log_error(char *buf, unsigned int err_type, int fatal) ppc_md.log_error (buf, err_type, fatal); } +void generic_suspend_disable_irqs (void ); +void generic_suspend_enable_irqs (void );
And do they really need to be declared here?
josh
Quoted 15 lines · Josh Boyer
Quoted 15 lines
arch/powerpc/kernel/time.c +15
-0+#ifdef CONFIG_SUSPEND +void generic_suspend_disable_irqs (void ) +{ +preempt_disable (); + +/* Disable the decrementer, so that it doesn't interfere +* with suspending. +*/ + +set_dec (0x7fffffff ); +hard_irq_disable (); +set_dec (0x7fffffff ); +} + +void generic_suspend_enable_irqs (void )
Quoted 8 lines
arch/powerpc/kernel/time.c void wakeup_decrementer(void) · line 670
Quoted 6 lines
arch/powerpc/kernel/time.c +6
-0+{ +wakeup_decrementer (); + +local_irq_enable (); +preempt_enable (); +}Should these be static functions? Seems so...
I was asked to make them available to be called by non-generic implementations.
-Scott
Quoted 25 lines · Scott Wood
Quoted 15 lines · Josh Boyer
Quoted 15 lines
arch/powerpc/kernel/time.c +15
-0+#ifdef CONFIG_SUSPEND +void generic_suspend_disable_irqs (void ) +{ +preempt_disable (); + +/* Disable the decrementer, so that it doesn't interfere +* with suspending. +*/ + +set_dec (0x7fffffff ); +hard_irq_disable (); +set_dec (0x7fffffff ); +} + +void generic_suspend_enable_irqs (void )Quoted 8 lines
arch/powerpc/kernel/time.c void wakeup_decrementer(void) · line 670
Quoted 6 lines
arch/powerpc/kernel/time.c +6
-0+{ +wakeup_decrementer (); + +local_irq_enable (); +preempt_enable (); +}Should these be static functions? Seems so...
Quoted 2 lines
I was asked to make them available to be called by non-generic implementations.
Perhaps it's just me, but the names seem a bit _too_ generic if they aren't going to be static functions. Maybe I'm paranoid.
josh
Quoted 11 lines · Scott Wood
arch/powerpc/kernel/time.c +11
-0+#ifdef CONFIG_SUSPEND +void generic_suspend_disable_irqs (void ) +{ +preempt_disable (); + +/* Disable the decrementer, so that it doesn't interfere +* with suspending. +*/ + +set_dec (0x7fffffff ); +hard_irq_disable ();
arch/powerpc/kernel/time.c void wakeup_decrementer(void) · line 670
Quoted 10 lines
arch/powerpc/kernel/time.c +10
-0+set_dec (0x7fffffff ); +} + +void generic_suspend_enable_irqs (void ) +{ +wakeup_decrementer (); + +local_irq_enable (); +preempt_enable (); +}
Doh, that should be hard_irq_enable().
-Scott
Paulus,
is there any specific reason, why out of these 7 patches only the first one made it into the mainline? AFAICS, there has been only one comment, suggesting to replace printk with dev_err on two occasions in one of the patches...
Thanks
Guennadi
---
Guennadi Liakhovetski
Quoted 4 lines · Guennadi Liakhovetski
is there any specific reason, why out of these 7 patches only the first one made it into the mainline? AFAICS, there has been only one comment, suggesting to replace printk with
dev_erron two occasions in one of the patches...
A while ago Paul said on IRC he'd prefer to do the TLF_SLEEPING hack more like the soft IRQ disabling that 64-bit uses. I haven't yet had a chance to look into it, so the patch collects dust, despite the current implementation of TLF_SLEEPING working just fine.
-Scott
Quoted 10 lines · Scott Wood
Quoted 4 lines · Guennadi Liakhovetski
is there any specific reason, why out of these 7 patches only the first one made it into the mainline? AFAICS, there has been only one comment, suggesting to replace printk with
dev_erron two occasions in one of the patches...A while ago Paul said on IRC he'd prefer to do the
TLF_SLEEPINGhack more like the soft IRQ disabling that 64-bit uses. I haven't yet had a chance to look into it, so the patch collects dust, despite the current implementation ofTLF_SLEEPINGworking just fine.
I have taken a closer look at the TLF_SLEEPING patch and crystallized my thoughts about it a bit:
1. Too many ifdefs - it's only a few instructions extra, so if we're going to have the TLF_SLEEPING stuff we might as well have it unconditionally.
2. It seems convoluted to me to go through transfer_to_handler_cont and ret_from_except when we could just get out directly through fast_exception_return, given that we are not calling a handler. The only thing to watch out for there is that r7 and r8 haven't been modified (or have been restored if they have).
3. The style in all the assembly code is not to have spaces after commas separating instruction operands.
The untested patch below is what I was thinking of. If you'd like to try it out, I'd be interested to hear how it goes.
Paul.
arch/powerpc/kernel/entry_32.S +8 -0
@@ -146,6 +146,7 @@ transfer_to_handler: lwz r12,TI_LOCAL_FLAGS(r9) mtcrf 0x01,r12 bt- 31-TLF_NAPPING,4f + bt- 31-TLF_SLEEPING,7f #endif /* CONFIG_6xx */ .globl transfer_to_handler_cont transfer_to_handler_cont:
@@ -163,6 +164,13 @@ transfer_to_handler_cont: 4: rlwinm r12,r12,0,~_TLF_NAPPING stw r12,TI_LOCAL_FLAGS(r9) b power_save_6xx_restore + +7: rlwinm r12,r12,0,~TLF_SLEEPING + stw r12,TI_LOCAL_FLAGS(r9) + lwz r9,_MSR(r11) /* if sleeping, clear MSR.EE */ + rlwinm r9,r9,0,~MSR_EE + lwz r12,_LINK(r11) /* and return to address in LR */ + b fast_exception_return #endif /*
include/asm-powerpc/thread_info.h +2 -0
@@ -147,8 +147,10 @@ static inline struct thread_info *current_thread_info(void)/* Bits in local_flags */ /* Don't move TLF_NAPPING without adjusting the code in entry_32.S */ #define TLF_NAPPING 0/* idle thread enabled NAP mode */ +#define TLF_SLEEPING 1/* suspend code enabled SLEEP mode */ #define _TLF_NAPPING (1 << TLF_NAPPING) +#define _TLF_SLEEPING (1 << TLF_SLEEPING)#endif /* __KERNEL__ */
Quoted 1 line · I wrote:
+7: rlwinm r12,r12,0,~TLF_SLEEPING
That should be rlwinm r12,r12,0,~_TLF_SLEEPING (with the leading underscore), of course. Thanks to Stephen Rothwell for pointing that out.
Paul.
Quoted 26 lines · Paul Mackerras
Quoted 10 lines · Scott Wood
Quoted 4 lines · Guennadi Liakhovetski
is there any specific reason, why out of these 7 patches only the first one made it into the mainline? AFAICS, there has been only one comment, suggesting to replace printk with
dev_erron two occasions in one of the patches...A while ago Paul said on IRC he'd prefer to do the
TLF_SLEEPINGhack more like the soft IRQ disabling that 64-bit uses. I haven't yet had a chance to look into it, so the patch collects dust, despite the current implementation ofTLF_SLEEPINGworking just fine.I have taken a closer look at the
TLF_SLEEPINGpatch and crystallized my thoughts about it a bit:1. Too many ifdefs - it's only a few instructions extra, so if we're going to have the
TLF_SLEEPINGstuff we might as well have it unconditionally.2. It seems convoluted to me to go through
transfer_to_handler_contandret_from_exceptwhen we could just get out directly throughfast_exception_return, given that we are not calling a handler. The only thing to watch out for there is that r7 and r8 haven't been modified (or have been restored if they have).
Quoted 5 lines
3. The style in all the assembly code is not to have spaces after commas separating instruction operands.
The untested patch below is what I was thinking of. If you'd like to try it out, I'd be interested to hear how it goes.
The patch (with the _TLF_SLEEPING fix you mentioned in a later email) works for me. Shall I submit it "From: <you>" or would you prefer to post it yourself? But, I guess, you have to put your "S-o-b" under it yourself, don't you?
Thanks
Guennadi
---
Guennadi Liakhovetski
Quoted 2 lines · Guennadi Liakhovetski
The patch (with the
_TLF_SLEEPINGfix you mentioned in a later email) works for me.
Great, thanks.
Quoted 3 lines
Shall I submit it "From: <you>" or would you prefer to post
it yourself? But, I guess, you have to put your "S-o-b" under it yourself, don't you?
I'll post it as a stand-alone patch with my S-o-b.
Paul.
Quoted 2 lines · Paul Mackerras
3. The style in all the assembly code is not to have spaces after commas separating instruction operands.
I'll do that if that's what is prefered, but how did that come about as the style used? It's different from what we do in C, and adding the space helps readability in asm as well...
Quoted 2 lines
The untested patch below is what I was thinking of. If you'd like to try it out, I'd be interested to hear how it goes.
It works for me.
-Scott
Quoted 5 lines
Quoted 2 lines
3. The style in all the assembly code is not to have spaces after commas separating instruction operands.
I'll do that if that's what is prefered, but how did that come about as the style used? It's different from what we do in C,
But this isn't C code, it's assembler code. PowerPC assembler code (and most other assembler code) doesn't use spaces here usually.
Quoted 2 lines
and adding the
space helps readability in asm as well...
Many people disagree ;-)
Anyway, it's better to keep a consistent style, whatever that style is, don't you agree?
Segher
Quoted 7 lines · Segher Boessenkool
Quoted 5 lines
Quoted 2 lines
3. The style in all the assembly code is not to have spaces after commas separating instruction operands.
I'll do that if that's what is prefered, but how did that come about as the style used? It's different from what we do in C,
But this isn't C code, it's assembler code.
Enh. It's a comma-delineated list in both cases.
Quoted 2 lines
PowerPC assembler code (and most other assembler code) doesn't use spaces here usually.
Looking at other arches, it seems pretty evenly split.
Quoted 3 lines
Quoted 1 line
and adding the space helps readability in asm as well...
Many people disagree ;-)
Compressionhelpsreadability?
Quoted 2 lines
Anyway, it's better to keep a consistent style, whatever that style is, don't you agree?
Not if it's GNU-style. :-)
-Scott