Hi Kevin, On Thu, Jun 2, 2011 at 11:59 PM, Kevin Hilman <khilman@xxxxxx> wrote: > Lesly A M <leslyam@xxxxxx> writes: > >> Power bus message sequence for TWL4030 to enter sleep/wakeup/warm_reset. >> >> TWL4030 power scripts which can be used by different OMAP3 boards >> with the power companion chip (TWL4030 series). >> >> The twl4030 generic script can be used by any board file to update >> the power data in twl4030_platform_data. > > What about existing board files that are using their own scripts? > > On n900 for example, board-rx51-peripherals.c has its own custom > scripts which are registered at board init time. These are then > over written by your new driver which loads later. > > So first, can you verify if the n900 scripts can/should be replaced by > these generic ones. And second, please find a way for board files to > override these scripts at runtime instead of using a Kconfig. Customer boards have to modify the script for their board (may be using modem which is not present in SDP). Is it ok to add a flag to check whether the board file has already initialized the script. >> Since the TWL4030 power script has dependency with APIs in twl4030-power.c >> removing the __init for these APIs. > > I think separating this part out into a separate patch, with a better > description would make the rest of this patch much more readable. As it > is, it doesn't make much sense since all the script functions and data > are __init or __initdata. Ok > These changes also introduce a new complier warning: > > CC drivers/mfd/twl4030-power.o > /work/kernel/omap/pm/drivers/mfd/twl4030-power.c: In function 'twl4030_power_init': > /work/kernel/omap/pm/drivers/mfd/twl4030-power.c:457:5: warning: 'err' may be used uninitialized in this function > /work/kernel/omap/pm/drivers/mfd/twl4030-power.c:171:6: note: 'err' was declared here Ok, I will fix it > >> For more information please see: >> http://omapedia.org/wiki/TWL4030_power_scripts > > This is an excellent wiki, thank for the detailed description! > >> Signed-off-by: Lesly A M <leslyam@xxxxxx> >> Cc: Nishanth Menon <nm@xxxxxx> >> Cc: David Derrick <dderrick@xxxxxx> >> Cc: Samuel Ortiz <sameo@xxxxxxxxxxxxxxx> >> --- >> arch/arm/configs/omap2plus_defconfig | 1 + >> arch/arm/mach-omap2/devices.c | 15 ++ >> drivers/mfd/Kconfig | 11 + >> drivers/mfd/Makefile | 1 + >> drivers/mfd/twl4030-power.c | 31 ++-- >> drivers/mfd/twl4030-script-omap.c | 373 ++++++++++++++++++++++++++++++++++ >> include/linux/i2c/twl.h | 41 ++++- >> 7 files changed, 454 insertions(+), 19 deletions(-) >> create mode 100644 drivers/mfd/twl4030-script-omap.c >> >> diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig >> index 076db52..d9b9858 100644 >> --- a/arch/arm/configs/omap2plus_defconfig >> +++ b/arch/arm/configs/omap2plus_defconfig >> @@ -184,6 +184,7 @@ CONFIG_TWL4030_WATCHDOG=y >> CONFIG_MENELAUS=y >> CONFIG_TWL4030_CORE=y >> CONFIG_TWL4030_POWER=y >> +CONFIG_TWL4030_SCRIPT=m >> CONFIG_REGULATOR=y >> CONFIG_REGULATOR_TWL4030=y >> CONFIG_REGULATOR_TPS65023=y >> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c >> index 7b85585..7653329 100644 >> --- a/arch/arm/mach-omap2/devices.c >> +++ b/arch/arm/mach-omap2/devices.c >> @@ -329,6 +329,20 @@ static void omap_init_audio(void) >> static inline void omap_init_audio(void) {} >> #endif >> >> +#ifdef CONFIG_ARCH_OMAP3 > > Shouldn't this depend on CONFIG_TWL4030_SCRIPT ? Ok >> +static struct platform_device omap_twl4030_script = { >> + .name = "twl4030_script", >> + .id = -1, >> +}; >> + >> +static void omap_init_twl4030_script(void) >> +{ >> + platform_device_register(&omap_twl4030_script); >> +} >> +#else >> +static inline void omap_init_twl4030_script(void) {} >> +#endif > > I guess this gets to the debate about whether these scripts should be in > drivers/mfd or in arch/arm/mach-omap2, but wherever the script data > lives, this platform_device definition and register should be also. > > IOW, there's not a clean separation between driver and device currently. > IMO, The platform_driver part should just be part of twl4030-power.c, > and all the scripts and platform_device creation/registration should be > part of the script file. > > As far as where the script device data should go, I'd vote for > drivers/mfd, where it can be compiled as a module along with the rest of > the twl4030 code. Any final conclusion ? >> #if defined(CONFIG_SPI_OMAP24XX) || defined(CONFIG_SPI_OMAP24XX_MODULE) >> >> #include <plat/mcspi.h> >> @@ -691,6 +705,7 @@ static int __init omap2_init_devices(void) >> omap_init_sham(); >> omap_init_aes(); >> omap_init_vout(); >> + omap_init_twl4030_script(); >> >> return 0; >> } >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> index 3ed3ff0..bed88ce 100644 >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -204,6 +204,17 @@ config TWL4030_POWER >> and load scripts controlling which resources are switched off/on >> or reset when a sleep, wakeup or warm reset event occurs. >> >> +config TWL4030_SCRIPT >> + tristate "Support TWL4030 script for OMAP3 boards" >> + depends on TWL4030_CORE && TWL4030_POWER >> + help >> + Say yes here if you want to use the twl4030 power scripts >> + for OMAP3 boards. Power bus message sequence for >> + TWL4030 to enter sleep/wakeup/warm_reset. >> + >> + TWL4030 power scripts which can be used by different >> + OMAP3 boards with the power companion chip (TWL4030 series). >> + > > Why do we need another Kconfig for this? It should be enabled and > overridden at runtime via board files. Kconfig option was added when script was changed to insert-able module, which was suggested by Tony as comment for last version. Thanks & Regards, Lesly -- 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
