Re: [PATCH] OMAP3: PM: Add the wakeup source driver, v3
On Thu, Apr 30, 2009 at 11:21 PM, Kevin Hilman khil...@deeprootsystems.com wrote: 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. That's what I assumed was happening. 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? ;) Yes. The WKST registers are already being read in the handler so they can be properly cleared. All you are adding is the saving of them. In addition, you should not enter idle between the time the system wakes from resume and your resume handler runs so you should be able to get the correct WKST values. OK, Your idea is more reasonable. I modified as you said. thanks. 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. That sounds OK. The current mux layer is lacking any knowledge of the wake bits in the PADCONF regs, so I'd be interested in any ideas you have of adding that support. I added my ideas about CONTROL_PADCONF_x into the new version of wake source driver. I will send it soon. Please review this new version again and feel free to give your opinion. Regards, Kyuwon (규원) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3: PM: Add the wakeup source driver, v3
Hi Kevin, Thank you for showing steady interest in this driver. On Tue, Apr 21, 2009 at 9:15 AM, Kevin Hilman khil...@deeprootsystems.com 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_SHIFT3 #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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3: PM: Add the wakeup source driver, v3
Kyuwon, Kim Kyuwon chamm...@gmail.com writes: Hi Kevin, Thank you for showing steady interest in this driver. On Tue, Apr 21, 2009 at 9:15 AM, Kevin Hilman khil...@deeprootsystems.com 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. Extending omapdev would be fine. 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. That's what I assumed was happening. 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? ;) Yes. The WKST registers are already being read in the handler so they can be properly cleared. All you are adding is the saving of them. In addition, you should not enter idle between the time the system wakes from resume and your resume handler runs so you should be able to get the correct WKST values. 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. That sounds OK. The current mux layer is lacking any knowledge of the wake bits in the PADCONF regs, so I'd be interested in any ideas you have of adding that support. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3: PM: Add the wakeup source driver, v3
Kim Kyuwon chamm...@gmail.com writes:
Sometimes, it is necessary to find out what does wake up my board
from suspend?. Notifying wake-up source feature may be used to blame
unexpected wake-up events which increase power consumption. And user
mode applications can act smartly according to the wake-up event from
Suspend-to-RAM state to minimize power consumption. Note that this
driver can't inform wake-up events from idle state. This driver uses
sysfs interface to give information to user mode applications like:
cat /sys/power/omap_resume_irq
cat /sys/power/omap_resume_event
This driver also privides the unified GPIO wake-up source
configuration. specific GPIO settings in the board files are:
/* Wakeup source configuration */
static struct gpio_wake boardname_gpio_wake[] = {
{ 23, IRQF_TRIGGER_RISING,BT_WAKEUP,1},
{ 24, IRQF_TRIGGER_RISING,USB_DETECT, 1},
};
static struct omap_wake_platform_data boardname_wake_data = {
.gpio_wakes = boardname_gpio_wake,
.gpio_wake_num = ARRAY_SIZE(boardname_gpio_wake),
};
static struct platform_device boardname_wakeup = {
.name = omap-wake,
.id = -1,
.dev= {
.platform_data = boardname_wake_data,
},
};
The patch adds Kconfig options OMAP34xx wakeup source support under
System type-TI OMAP implementations menu.
Signed-off-by: Kim Kyuwon q1@samsung.com
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.
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.
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.
---
arch/arm/mach-omap2/Makefile |1 +
arch/arm/mach-omap2/irq.c | 21 +-
arch/arm/mach-omap2/prcm-common.h |4 +
arch/arm/mach-omap2/prm-regbits-34xx.h |5 +
arch/arm/mach-omap2/wake34xx.c | 681
arch/arm/plat-omap/Kconfig |9 +
arch/arm/plat-omap/include/mach/irqs.h |4 +
arch/arm/plat-omap/include/mach/wake.h | 30 ++
8 files changed, 752 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/mach-omap2/wake34xx.c
create mode 100644 arch/arm/plat-omap/include/mach/wake.h
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index e693efd..4d7dbca 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_ARCH_OMAP2)+= pm24xx.o
obj-$(CONFIG_ARCH_OMAP24XX) += sleep24xx.o
obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o cpuidle34xx.o
obj-$(CONFIG_PM_DEBUG) += pm-debug.o
+obj-$(CONFIG_OMAP_WAKE) += wake34xx.o
endif
# SmartReflex driver
diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 700fc3d..18ac725 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -33,9 +33,6 @@
#define INTC_MIR_SET00x008c
#define INTC_PENDING_IRQ00x0098
-/* Number of IRQ state bits in each MIR register */
-#define IRQ_BITS_PER_REG 32
-
/*
* OMAP2 has a number of different interrupt controllers, each interrupt
* controller is identified as its own bank. Register definitions are
@@ -193,6 +190,24 @@ int omap_irq_pending(void)
return 0;
}
+void omap_get_pending_irqs(u32 *pending_irqs, unsigned len)
+{
+ int i, j = 0;
+
+ for (i = 0; i ARRAY_SIZE(irq_banks); i++) {
+ struct omap_irq_bank *bank = irq_banks + i;
+ int irq;
+
+ for (irq = 0; irq bank-nr_irqs j len;
+ irq += IRQ_BITS_PER_REG) {
+ int offset = irq (~(IRQ_BITS_PER_REG - 1));
+
+ pending_irqs[j++] = intc_bank_read_reg(bank,
+ (INTC_PENDING_IRQ0 + offset));
+ }
+ }
+}
+
void
Re: [PATCH] OMAP3: PM: Add the wakeup source driver, v3
Hi Kevin,
Have you had chance to review this new version of wakeup driver? :)
Regards,
Kyuwon
On Fri, Apr 3, 2009 at 7:43 PM, Kim Kyuwon chamm...@gmail.com wrote:
Sometimes, it is necessary to find out what does wake up my board
from suspend?. Notifying wake-up source feature may be used to blame
unexpected wake-up events which increase power consumption. And user
mode applications can act smartly according to the wake-up event from
Suspend-to-RAM state to minimize power consumption. Note that this
driver can't inform wake-up events from idle state. This driver uses
sysfs interface to give information to user mode applications like:
cat /sys/power/omap_resume_irq
cat /sys/power/omap_resume_event
This driver also privides the unified GPIO wake-up source
configuration. specific GPIO settings in the board files are:
/* Wakeup source configuration */
static struct gpio_wake boardname_gpio_wake[] = {
{ 23, IRQF_TRIGGER_RISING,BT_WAKEUP,1},
{ 24, IRQF_TRIGGER_RISING,USB_DETECT, 1},
};
static struct omap_wake_platform_data boardname_wake_data = {
.gpio_wakes = boardname_gpio_wake,
.gpio_wake_num = ARRAY_SIZE(boardname_gpio_wake),
};
static struct platform_device boardname_wakeup = {
.name = omap-wake,
.id = -1,
.dev= {
.platform_data = boardname_wake_data,
},
};
The patch adds Kconfig options OMAP34xx wakeup source support under
System type-TI OMAP implementations menu.
Signed-off-by: Kim Kyuwon q1@samsung.com
---
arch/arm/mach-omap2/Makefile |1 +
arch/arm/mach-omap2/irq.c | 21 +-
arch/arm/mach-omap2/prcm-common.h |4 +
arch/arm/mach-omap2/prm-regbits-34xx.h |5 +
arch/arm/mach-omap2/wake34xx.c | 681
arch/arm/plat-omap/Kconfig |9 +
arch/arm/plat-omap/include/mach/irqs.h |4 +
arch/arm/plat-omap/include/mach/wake.h | 30 ++
8 files changed, 752 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/mach-omap2/wake34xx.c
create mode 100644 arch/arm/plat-omap/include/mach/wake.h
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index e693efd..4d7dbca 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o
obj-$(CONFIG_ARCH_OMAP24XX)+= sleep24xx.o
obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o cpuidle34xx.o
obj-$(CONFIG_PM_DEBUG) += pm-debug.o
+obj-$(CONFIG_OMAP_WAKE)+= wake34xx.o
endif
# SmartReflex driver
diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 700fc3d..18ac725 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -33,9 +33,6 @@
#define INTC_MIR_SET0 0x008c
#define INTC_PENDING_IRQ0 0x0098
-/* Number of IRQ state bits in each MIR register */
-#define IRQ_BITS_PER_REG 32
-
/*
* OMAP2 has a number of different interrupt controllers, each interrupt
* controller is identified as its own bank. Register definitions are
@@ -193,6 +190,24 @@ int omap_irq_pending(void)
return 0;
}
+void omap_get_pending_irqs(u32 *pending_irqs, unsigned len)
+{
+ int i, j = 0;
+
+ for (i = 0; i ARRAY_SIZE(irq_banks); i++) {
+ struct omap_irq_bank *bank = irq_banks + i;
+ int irq;
+
+ for (irq = 0; irq bank-nr_irqs j len;
+ irq += IRQ_BITS_PER_REG) {
+ int offset = irq (~(IRQ_BITS_PER_REG - 1));
+
+ pending_irqs[j++] = intc_bank_read_reg(bank,
+ (INTC_PENDING_IRQ0 + offset));
+ }
+ }
+}
+
void __init omap_init_irq(void)
{
unsigned long nr_of_irqs = 0;
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_SHIFT3
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
