Hi Kevin, Thank you for showing steady interest in this driver. On Tue, Apr 21, 2009 at 9:15 AM, Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx> wrote: > > Hi Kyuwon, > > While I still like the concept of this driver, I'm still not quite > happy about how it is implemented for various reasons. Most of which > have to do with the fact that this driver does many things that really > should be the job of other layers. In particular, the list of > omap3_wake_events is a bit troubling. It really should be handled by > the omapdev layer. You'll see that you are duplicating the data that > is handled there. Rather, we should just extend the omapdev data to > handle the wakeup events for that device. If you allow me to insert a new field whose name is "mask" in the omapdev struct, I think I can extend the omapdev to handle the wakeup events. > Also, I still think the WKST register reading should be done in the > PRCM interrupt handler. In your previous attempt, you were seeing a > bunch of non-device wakeup related interrupts. Can you try again > with the attached patch (thanks to Paul Walmseley) which should help > us debug why you were seeing spurious PRCM interrupts. First of all, sorry for giving you the wrong information in the previous mail. I found that we actually have configured GPTIMER12 as the system timer. And I tried with the attached patch, but I can't see any debug message. However even now, PRCM interrupt handler is invoked quite often due to the system timer. As far as I know, after Peter's [OMAP3: PM: CPUidle: Add new lower-latency C1 state] patch is applied, the system is in 'wfi' state in every idle state. So whenever the system timer wake up the system from idle, I think PRCM interrupt occurs. Do you think still the WKST register should be read in the PRCM interrupt handler? ;) > Also, I know we discussed this before, but I think the GPIO wakeup > source stuff really belongs in a separate patch, and if you think it > is still useful, and cannot be done by just enabling a GPIO IRQ from > the board file, I suggest you propose a patch to the generic GPIO > layer to add this interface. OK, I will remove this GPIO wakeup feature. But I want to know more detailed information about wakeup event . So, instead of using the GPIO wakeup, I'm planning using WAKEUPEVENT bit in CONTROL_PADCONF_x registers. >> diff --git a/arch/arm/mach-omap2/prcm-common.h >> b/arch/arm/mach-omap2/prcm-common.h >> index cb1ae84..1f340aa 100644 >> --- a/arch/arm/mach-omap2/prcm-common.h >> +++ b/arch/arm/mach-omap2/prcm-common.h >> @@ -273,6 +273,10 @@ >> #define OMAP3430_ST_D2D_SHIFT 3 >> #define OMAP3430_ST_D2D_MASK (1 << 3) >> >> +/* PM_WKST3_CORE, CM_IDLEST3_CORE shared bits */ >> +#define OMAP3430_ST_USBTLL_SHIFT 2 >> +#define OMAP3430_ST_USBTLL_MASK (1 << 2) >> + >> /* CM_FCLKEN_WKUP, CM_ICLKEN_WKUP, PM_WKEN_WKUP shared bits */ >> #define OMAP3430_EN_GPIO1 (1 << 3) >> #define OMAP3430_EN_GPIO1_SHIFT 3 >> diff --git a/arch/arm/mach-omap2/prm-regbits-34xx.h >> b/arch/arm/mach-omap2/prm-regbits-34xx.h >> index 06fee29..f0a6395 100644 >> --- a/arch/arm/mach-omap2/prm-regbits-34xx.h >> +++ b/arch/arm/mach-omap2/prm-regbits-34xx.h >> @@ -332,6 +332,8 @@ >> /* PM_IVA2GRPSEL1_CORE specific bits */ >> >> /* PM_WKST1_CORE specific bits */ >> +#define OMAP3430_ST_MMC3_SHIFT 30 >> +#define OMAP3430_ST_MMC3_MASK (1 << 30) >> >> /* PM_PWSTCTRL_CORE specific bits */ >> #define OMAP3430_MEM2ONSTATE_SHIFT 18 >> @@ -432,6 +434,9 @@ >> >> /* PM_PREPWSTST_PER specific bits */ >> >> +/* PM_WKST_USBHOST specific bits */ >> +#define OMAP3430_ST_USBHOST (1 << 0) >> + >> /* RM_RSTST_EMU specific bits */ > > All these new bit defines should all be 3430ES2_*. OK, I will fix it. Thanks & Regards, Kyuwon -- 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
