On 07/04/2017 08:14 AM, Johan Hovold wrote: > On Fri, May 19, 2017 at 03:04:36PM -0500, Dave Gerlach wrote: >> Most of the PM code needed for am335x and am437x can be moved into a >> module under drivers but some core code must remain in mach-omap2 at the >> moment. This includes some internal clockdomain APIs and low-level ARM >> APIs which are also not exported for use by modules. >> >> Implement a few functions that handle these low-level platform >> operations can be passed to the pm33xx module through the use of >> platform data. >> >> In addition to this, to be able to share data structures between C and >> the sleep33xx and sleep43xx assembly code, we can automatically generate >> all of the C struct member offsets and sizes as macros by making use of >> the ARM asm-offsets file. In the same header that we define our data >> structures in we also define all the macros in an inline function and by >> adding a call to this in the asm_offsets file all macros are properly >> generated and available to the assembly code without cluttering up the >> asm-offsets file. >> >> Signed-off-by: Dave Gerlach <d-gerlach@xxxxxx> >> --- > >> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h >> index b668719b9b25..2f9649b89053 100644 >> --- a/arch/arm/mach-omap2/pm.h >> +++ b/arch/arm/mach-omap2/pm.h >> @@ -81,6 +81,11 @@ extern unsigned int omap3_do_wfi_sz; >> /* ... and its pointer from SRAM after copy */ >> extern void (*omap3_do_wfi_sram)(void); >> >> +struct am33xx_pm_platform_data *am33xx_pm_get_pdata(void); > > This one is not used outside of pm33xx-core.c so can now be static, and > this declaration can be dropped. Yes you are correct, seems I missed this in a refactor. > >> diff --git a/arch/arm/mach-omap2/pm33xx-core.c b/arch/arm/mach-omap2/pm33xx-core.c >> new file mode 100644 >> index 000000000000..c84ffc4de2e9 >> --- /dev/null >> +++ b/arch/arm/mach-omap2/pm33xx-core.c >> @@ -0,0 +1,181 @@ >> +/* >> + * AM33XX Arch Power Management Routines >> + * >> + * Copyright (C) 2016-2017 Texas Instruments Incorporated - http://www.ti.com/ >> + * Dave Gerlach >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation version 2. >> + * >> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any >> + * kind, whether express or implied; without even the implied warranty >> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include <asm/smp_scu.h> > > I get a compilation error here when compiling for am335x without > CONFIG_HAVE_ARM_SCU due to a missing errno.h include: > > In file included from /home/johan/work/omicron/src/linux/arch/arm/mach-omap2/pm33xx-core.c:17:0: > /home/johan/work/omicron/src/linux/arch/arm/include/asm/smp_scu.h: In function 'scu_power_mode': > /home/johan/work/omicron/src/linux/arch/arm/include/asm/smp_scu.h:36:10: error: 'EINVAL' undeclared (first use in this function) > return -EINVAL; > ^ > /home/johan/work/omicron/src/linux/arch/arm/include/asm/smp_scu.h:36:10: note: each undeclared identifier is reported only once for each function it appears in > > This is arguably a bug in the header, which I'm submitting a fix for, > but you should include errno.h above anyway as you use its definitions > below as well. > Yes ok, good catch. >> +#include <asm/suspend.h> >> +#include <linux/platform_data/pm33xx.h> > > <snip> > >> diff --git a/include/linux/platform_data/pm33xx.h b/include/linux/platform_data/pm33xx.h >> new file mode 100644 >> index 000000000000..c191ab681093 >> --- /dev/null >> +++ b/include/linux/platform_data/pm33xx.h >> @@ -0,0 +1,69 @@ >> +/* >> + * TI pm33xx platform data >> + * >> + * Copyright (C) 2016-2017 Texas Instruments, Inc. >> + * Dave Gerlach <d-gerlach@xxxxxx> >> + * >> + * 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. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#ifndef _LINUX_PLATFORM_DATA_PM33XX_H >> +#define _LINUX_PLATFORM_DATA_PM33XX_H >> + > > And here you should add a linux/types.h include to make this header > self-contained. Right now you are depending on the scu header to pull in > the types for pm33xx-core.c above. > Ok will do. Thanks for the comments. Regards, Dave >> +#include <linux/kbuild.h> >> + >> +#ifndef __ASSEMBLER__ >> +struct am33xx_pm_sram_addr { >> + void (*do_wfi)(void); >> + unsigned long *do_wfi_sz; >> + unsigned long *resume_offset; >> + unsigned long *emif_sram_table; >> + unsigned long *ro_sram_data; >> +}; >> + >> +struct am33xx_pm_platform_data { >> + int (*init)(void); >> + int (*soc_suspend)(unsigned int state, int (*fn)(unsigned long)); >> + struct am33xx_pm_sram_addr *(*get_sram_addrs)(void); >> +}; >> + >> +struct am33xx_pm_sram_data { >> + u32 wfi_flags; >> + u32 l2_aux_ctrl_val; >> + u32 l2_prefetch_ctrl_val; >> +}; >> + >> +struct am33xx_pm_ro_sram_data { >> + u32 amx3_pm_sram_data_virt; >> + u32 amx3_pm_sram_data_phys; >> +}; > > Johan > -- 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