Patchwork [V2,1/6] ARM: tegra: add pending SGI checking API

login
register
mail settings
Submitter Joseph Lo
Date Dec. 5, 2012, 10:01 a.m.
Message ID <1354701715-24150-2-git-send-email-josephl@nvidia.com>
Download mbox | patch
Permalink /patch/203805/
State Superseded, archived
Headers show

Comments

Joseph Lo - Dec. 5, 2012, 10:01 a.m.
The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
include the power of GIC. That caused the SGI (Software Generated
Interrupt) been lost. Because the SGI can't wake up the CPU that in
the "powered-down" CPU idle mode. We need to check if there is any
pending SGI when go into "powered-down" CPU idle mode. This is important
especially when applying the coupled cpuidle framework into "power-down"
cpuidle dirver. Because the coupled cpuidle framework may have the
chance that misses IPI_SINGLE_FUNC handling sometimes.

For the PPI or SPI, something like the legacy peripheral interrupt. It
still can be maintained by Tegra legacy interrupt controller. If there
is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
CPU can be woken up immediately. So we don't need to take care the same
situation for PPI or SPI.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
V2:
* new in V2
---
 arch/arm/mach-tegra/irq.c |   17 +++++++++++++++--
 arch/arm/mach-tegra/irq.h |   22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/mach-tegra/irq.h
Stephen Warren - Dec. 5, 2012, 10:09 p.m.
On 12/05/2012 03:01 AM, Joseph Lo wrote:
> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> include the power of GIC. That caused the SGI (Software Generated
> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> the "powered-down" CPU idle mode. We need to check if there is any
> pending SGI when go into "powered-down" CPU idle mode. This is important
> especially when applying the coupled cpuidle framework into "power-down"
> cpuidle dirver. Because the coupled cpuidle framework may have the
> chance that misses IPI_SINGLE_FUNC handling sometimes.
> 
> For the PPI or SPI, something like the legacy peripheral interrupt. It
> still can be maintained by Tegra legacy interrupt controller. If there
> is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
> CPU can be woken up immediately. So we don't need to take care the same
> situation for PPI or SPI.

Is this feature something that can/should be added to the core GIC
driver, rather than something custom in the Tegra code?

Part of the reason I ask is that I'd like to avoid any more:

static void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);

since that requires static page tables to be set up, whereas I'd like to
reduce, as much as possible, the set of pages Tegra maps statically.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joseph Lo - Dec. 6, 2012, 7:04 a.m.
On Thu, 2012-12-06 at 06:09 +0800, Stephen Warren wrote:
> On 12/05/2012 03:01 AM, Joseph Lo wrote:
> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > include the power of GIC. That caused the SGI (Software Generated
> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > the "powered-down" CPU idle mode. We need to check if there is any
> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > especially when applying the coupled cpuidle framework into "power-down"
> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> > 
> > For the PPI or SPI, something like the legacy peripheral interrupt. It
> > still can be maintained by Tegra legacy interrupt controller. If there
> > is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
> > CPU can be woken up immediately. So we don't need to take care the same
> > situation for PPI or SPI.
> 
> Is this feature something that can/should be added to the core GIC
> driver, rather than something custom in the Tegra code?
> 
This function is SoC specific code not a generic common code even I
modify it to more generic for checking the pending irq (SGI, PPI and
SPI). Different SoC had different design about it. For ex, some SoC only
put CPU core to power saving mode not include GIC, or there is another
irq controller can handle the case when CPU go into power saving mode.
Differenc SoC had different usage here (some need to check all pending
irq, some need to check SGI only and some even no need to consider
this).

> Part of the reason I ask is that I'd like to avoid any more:
> 
> static void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
> 
> since that requires static page tables to be set up, whereas I'd like to
> reduce, as much as possible, the set of pages Tegra maps statically.

I can move this into the function as a temp variable.

Thanks,
Joseph

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Dec. 6, 2012, 6:52 p.m.
On 12/06/2012 12:04 AM, Joseph Lo wrote:
> On Thu, 2012-12-06 at 06:09 +0800, Stephen Warren wrote:
>> On 12/05/2012 03:01 AM, Joseph Lo wrote:
>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>> include the power of GIC. That caused the SGI (Software Generated
>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>> the "powered-down" CPU idle mode. We need to check if there is any
>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>> especially when applying the coupled cpuidle framework into "power-down"
>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>
>>> For the PPI or SPI, something like the legacy peripheral interrupt. It
>>> still can be maintained by Tegra legacy interrupt controller. If there
>>> is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
>>> CPU can be woken up immediately. So we don't need to take care the same
>>> situation for PPI or SPI.
>>
>> Is this feature something that can/should be added to the core GIC
>> driver, rather than something custom in the Tegra code?
>>
> This function is SoC specific code not a generic common code even I
> modify it to more generic for checking the pending irq (SGI, PPI and
> SPI). Different SoC had different design about it. For ex, some SoC only
> put CPU core to power saving mode not include GIC, or there is another
> irq controller can handle the case when CPU go into power saving mode.
> Differenc SoC had different usage here (some need to check all pending
> irq, some need to check SGI only and some even no need to consider
> this).

Hmmm. OK.

>> Part of the reason I ask is that I'd like to avoid any more:
>>
>> static void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
>>
>> since that requires static page tables to be set up, whereas I'd like to
>> reduce, as much as possible, the set of pages Tegra maps statically.
> 
> I can move this into the function as a temp variable.

Well, the issue here is use of the IO_ADDRESS() macro at all; ioremap()
at run-time would be better, which I imagine is what the core GIC driver
does when intialized from DT. Still, I suppose there are many instance
of IO_ADDRESS() in the mach-tegra directory right now, so adding one
more won't hurt too much; we still need to do a big pass to get rid of
them sometime if possible.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
index b7886f1..3d2483a 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/arch/arm/mach-tegra/irq.c
@@ -45,7 +45,10 @@ 
 
 #define FIRST_LEGACY_IRQ 32
 
+#define SGI_MASK 0xFFFF
+
 static int num_ictlrs;
+static void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
 
 static void __iomem *ictlr_reg_base[] = {
 	IO_ADDRESS(TEGRA_PRIMARY_ICTLR_BASE),
@@ -55,6 +58,18 @@  static void __iomem *ictlr_reg_base[] = {
 	IO_ADDRESS(TEGRA_QUINARY_ICTLR_BASE),
 };
 
+bool tegra_pending_sgi(void)
+{
+	u32 pending_set;
+
+	pending_set = readl_relaxed(distbase + GIC_DIST_PENDING_SET);
+
+	if (pending_set & SGI_MASK)
+		return true;
+
+	return false;
+}
+
 static inline void tegra_irq_write_mask(unsigned int irq, unsigned long reg)
 {
 	void __iomem *base;
@@ -114,9 +129,7 @@  static int tegra_retrigger(struct irq_data *d)
 void __init tegra_init_irq(void)
 {
 	int i;
-	void __iomem *distbase;
 
-	distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
 	num_ictlrs = readl_relaxed(distbase + GIC_DIST_CTR) & 0x1f;
 
 	if (num_ictlrs > ARRAY_SIZE(ictlr_reg_base)) {
diff --git a/arch/arm/mach-tegra/irq.h b/arch/arm/mach-tegra/irq.h
new file mode 100644
index 0000000..5142649
--- /dev/null
+++ b/arch/arm/mach-tegra/irq.h
@@ -0,0 +1,22 @@ 
+/*
+ * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __TEGRA_IRQ_H
+#define __TEGRA_IRQ_H
+
+bool tegra_pending_sgi(void);
+
+#endif