"Manuel, Lesly Arackal" <leslyam@xxxxxx> writes: > 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. Sounds fine. >>> 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. Yes, it needs to be a loadable module, but doesn't need Kconfig. IMO, it should just be build whenever CONFIG_TWL4030_POWER=y && CONFIG_ARCH_OMAP2PLUS=y Adding another build option means needing to ensure more build-option testing coverage. 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
