All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] ARM: CSR: add rtciobrg and PM support
@ 2011-08-23  6:15 Barry Song
  2011-08-23  6:15 ` [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII Barry Song
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Barry Song @ 2011-08-23  6:15 UTC (permalink / raw)
  To: linux-arm-kernel

rtciobrg is the bridge between the RTC clock domain and the CPU interface
clock domain. ARM access the register of SYSRTC, GPSRTC and PWRC through
this module.
Then PM controller depends on rtciobrg, we access PM through rtciobrg.

Barry Song (1):
  ARM: L2X0: move l2x0_init out of .init section

Rongjun Ying (2):
  ARM: CSR: add PM sleep entry for SiRFprimaII
  ARM: CSR: add PM suspend/resume entries for SiRFprimaII L2 cache

Zhiwu Song (1):
  ARM: CSR: add rtc i/o bridge interface for SiRFprimaII

 arch/arm/boot/dts/prima2-cb.dts            |    2 +-
 arch/arm/include/asm/hardware/cache-l2x0.h |    2 +-
 arch/arm/mach-prima2/Makefile              |    1 +
 arch/arm/mach-prima2/l2x0.c                |   82 +++++++++++----
 arch/arm/mach-prima2/pm.c                  |  158 ++++++++++++++++++++++++++++
 arch/arm/mach-prima2/pm.h                  |   31 ++++++
 arch/arm/mach-prima2/rtciobrg.c            |  136 ++++++++++++++++++++++++
 arch/arm/mach-prima2/sleep.S               |   61 +++++++++++
 arch/arm/mm/cache-l2x0.c                   |    2 +-
 include/linux/rtc/sirfsoc_rtciobrg.h       |   18 +++
 10 files changed, 468 insertions(+), 25 deletions(-)
 create mode 100644 arch/arm/mach-prima2/pm.c
 create mode 100644 arch/arm/mach-prima2/pm.h
 create mode 100644 arch/arm/mach-prima2/rtciobrg.c
 create mode 100644 arch/arm/mach-prima2/sleep.S
 create mode 100644 include/linux/rtc/sirfsoc_rtciobrg.h



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23  6:15 [PATCH v2 0/4] ARM: CSR: add rtciobrg and PM support Barry Song
@ 2011-08-23  6:15 ` Barry Song
  2011-08-23  8:48   ` Jamie Iles
  2011-08-23 16:01   ` Arnd Bergmann
  2011-08-23  6:15 ` [PATCH v2 2/4] ARM: CSR: add PM sleep entry " Barry Song
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 25+ messages in thread
From: Barry Song @ 2011-08-23  6:15 UTC (permalink / raw)
  To: linux-arm-kernel

From: Zhiwu Song <zhiwu.song@csr.com>

The module is a bridge between the RTC clock domain and the CPU interface
clock domain. ARM access the register of SYSRTC, GPSRTC and PWRC through
this module.

Signed-off-by: Zhiwu Song <zhiwu.song@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 -v2:
 handle feedback from Jamie Iles <jamie@jamieiles.com>
 make rtciobrg become a device driver
 make devices behind rtciobrg not be compatible with simple-bus
 replace EXPORT_SYMBOL by EXPORT_SYMBOL_GPL

 arch/arm/boot/dts/prima2-cb.dts      |    2 +-
 arch/arm/mach-prima2/Makefile        |    1 +
 arch/arm/mach-prima2/rtciobrg.c      |  136 ++++++++++++++++++++++++++++++++++
 include/linux/rtc/sirfsoc_rtciobrg.h |   18 +++++
 4 files changed, 156 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-prima2/rtciobrg.c
 create mode 100644 include/linux/rtc/sirfsoc_rtciobrg.h

diff --git a/arch/arm/boot/dts/prima2-cb.dts b/arch/arm/boot/dts/prima2-cb.dts
index 6fecc88..e5e9bc6 100644
--- a/arch/arm/boot/dts/prima2-cb.dts
+++ b/arch/arm/boot/dts/prima2-cb.dts
@@ -358,7 +358,7 @@
 		};
 
 		rtc-iobg {
-			compatible = "sirf,prima2-rtciobg", "simple-bus";
+			compatible = "sirf,prima2-rtciobg", "sirf-prima2-rtciobg-bus";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			reg = <0x80030000 0x10000>;
diff --git a/arch/arm/mach-prima2/Makefile b/arch/arm/mach-prima2/Makefile
index 7af7fc0..f49d70b 100644
--- a/arch/arm/mach-prima2/Makefile
+++ b/arch/arm/mach-prima2/Makefile
@@ -3,5 +3,6 @@ obj-y += irq.o
 obj-y += clock.o
 obj-y += rstc.o
 obj-y += prima2.o
+obj-y += rtciobrg.o
 obj-$(CONFIG_DEBUG_LL) += lluart.o
 obj-$(CONFIG_CACHE_L2X0) += l2x0.o
diff --git a/arch/arm/mach-prima2/rtciobrg.c b/arch/arm/mach-prima2/rtciobrg.c
new file mode 100644
index 0000000..1163e2c
--- /dev/null
+++ b/arch/arm/mach-prima2/rtciobrg.c
@@ -0,0 +1,136 @@
+/*
+ * RTC I/O Bridge interfaces for CSR SiRFprimaII
+ * ARM access the registers of SYSRTC, GPSRTC and PWRC through this module
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+
+#define SIRFSOC_CPUIOBRG_CTRL           0x00
+#define SIRFSOC_CPUIOBRG_WRBE           0x04
+#define SIRFSOC_CPUIOBRG_ADDR           0x08
+#define SIRFSOC_CPUIOBRG_DATA           0x0c
+
+void __iomem *sirfsoc_rtciobrg_base;
+static DEFINE_SPINLOCK(rtciobrg_lock);
+
+void sirfsoc_rtc_iobrg_wait_sync(void)
+{
+	while (readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL))
+		continue;
+}
+
+void sirfsoc_rtc_iobrg_besyncing(void)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&rtciobrg_lock, flags);
+
+	sirfsoc_rtc_iobrg_wait_sync();
+
+	spin_unlock_irqrestore(&rtciobrg_lock, flags);
+}
+EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_besyncing);
+
+u32 __sirfsoc_rtc_iobrg_readl(u32 addr)
+{
+	unsigned long val;
+
+	sirfsoc_rtc_iobrg_wait_sync();
+
+	writel_relaxed(0x00, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_WRBE);
+	writel_relaxed(addr, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_ADDR);
+	writel_relaxed(0x01, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL);
+
+	sirfsoc_rtc_iobrg_wait_sync();
+
+	val = readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_DATA);
+
+	return val;
+}
+
+u32 sirfsoc_rtc_iobrg_readl(u32 addr)
+{
+	unsigned long flags, val;
+
+	spin_lock_irqsave(&rtciobrg_lock, flags);
+
+	val = __sirfsoc_rtc_iobrg_readl(addr);
+
+	spin_unlock_irqrestore(&rtciobrg_lock, flags);
+
+	return val;
+}
+EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_readl);
+
+void sirfsoc_rtc_iobrg_pre_writel(u32 val, u32 addr)
+{
+	sirfsoc_rtc_iobrg_wait_sync();
+
+	writel_relaxed(0xf1, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_WRBE);
+	writel_relaxed(addr, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_ADDR);
+
+	writel_relaxed(val, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_DATA);
+}
+
+void sirfsoc_rtc_iobrg_writel(u32 val, u32 addr)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&rtciobrg_lock, flags);
+
+	sirfsoc_rtc_iobrg_pre_writel(val, addr);
+
+	writel_relaxed(0x01, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL);
+
+	sirfsoc_rtc_iobrg_wait_sync();
+
+	spin_unlock_irqrestore(&rtciobrg_lock, flags);
+}
+EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_writel);
+
+static struct of_device_id rtciobrg_ids[] = {
+	{ .compatible = "sirf,prima2-rtciobg" },
+	{}
+};
+
+static int __devinit sirfsoc_rtciobrg_probe(struct platform_device *op)
+{
+	struct device_node *np = op->dev.of_node;
+
+	sirfsoc_rtciobrg_base = of_iomap(np, 0);
+	if (!sirfsoc_rtciobrg_base)
+		panic("unable to map rtc iobrg registers\n");
+
+	return 0;
+}
+
+static struct platform_driver sirfsoc_rtciobrg_driver = {
+	.probe		= sirfsoc_rtciobrg_probe,
+	.driver = {
+		.name = "sirfsoc-rtciobrg",
+		.owner = THIS_MODULE,
+		.of_match_table	= rtciobrg_ids,
+	},
+};
+
+static int __init sirfsoc_rtciobrg_init(void)
+{
+	return platform_driver_register(&sirfsoc_rtciobrg_driver);
+}
+postcore_initcall(sirfsoc_rtciobrg_init);
+
+MODULE_AUTHOR("Zhiwu Song <zhiwu.song@csr.com>, "
+		"Barry Song <baohua.song@csr.com>");
+MODULE_DESCRIPTION("CSR SiRFprimaII rtc io bridge");
+MODULE_LICENSE("GPL");
+
diff --git a/include/linux/rtc/sirfsoc_rtciobrg.h b/include/linux/rtc/sirfsoc_rtciobrg.h
new file mode 100644
index 0000000..2c92e1c
--- /dev/null
+++ b/include/linux/rtc/sirfsoc_rtciobrg.h
@@ -0,0 +1,18 @@
+/*
+ * RTC I/O Bridge interfaces for CSR SiRFprimaII
+ * ARM access the registers of SYSRTC, GPSRTC and PWRC through this module
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+#ifndef _SIRFSOC_RTC_IOBRG_H_
+#define _SIRFSOC_RTC_IOBRG_H_
+
+extern void sirfsoc_rtc_iobrg_besyncing(void);
+
+extern u32 sirfsoc_rtc_iobrg_readl(u32 addr);
+
+extern void sirfsoc_rtc_iobrg_writel(u32 val, u32 addr);
+
+#endif
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-23  6:15 [PATCH v2 0/4] ARM: CSR: add rtciobrg and PM support Barry Song
  2011-08-23  6:15 ` [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII Barry Song
@ 2011-08-23  6:15 ` Barry Song
  2011-08-23  8:50   ` Jamie Iles
                     ` (2 more replies)
  2011-08-23  6:15 ` [PATCH v2 3/4] ARM: L2X0: move l2x0_init out of .init section Barry Song
  2011-08-23  6:15 ` [PATCH v2 4/4] ARM: CSR: add PM suspend/resume entries for SiRFprimaII L2 cache Barry Song
  3 siblings, 3 replies; 25+ messages in thread
From: Barry Song @ 2011-08-23  6:15 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rongjun Ying <rongjun.ying@csr.com>

Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 -v2:
 delete redundant ARM mode registers save/restore
 use generic cpu suspend
 move l2 cache suspend/resume codes out
 make memc become a device driver

 arch/arm/mach-prima2/pm.c    |  158 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-prima2/pm.h    |   31 ++++++++
 arch/arm/mach-prima2/sleep.S |   61 ++++++++++++++++
 3 files changed, 250 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-prima2/pm.c
 create mode 100644 arch/arm/mach-prima2/pm.h
 create mode 100644 arch/arm/mach-prima2/sleep.S

diff --git a/arch/arm/mach-prima2/pm.c b/arch/arm/mach-prima2/pm.c
new file mode 100644
index 0000000..75167df
--- /dev/null
+++ b/arch/arm/mach-prima2/pm.c
@@ -0,0 +1,158 @@
+/*
+ * power management entry for CSR SiRFprimaII
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/suspend.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/io.h>
+#include <linux/rtc/sirfsoc_rtciobrg.h>
+#include <asm/suspend.h>
+
+#include "pm.h"
+
+static u32 sirfsoc_pwrc_base;
+void __iomem *sirfsoc_memc_base;
+
+static void sirfsoc_set_wakeup_source(void)
+{
+        u32 pwr_trigger_en_reg;
+        pwr_trigger_en_reg = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
+		SIRFSOC_PWRC_TRIGGER_EN);
+#define X_ON_KEY_B (1 << 0)
+        sirfsoc_rtc_iobrg_writel(pwr_trigger_en_reg | X_ON_KEY_B,
+                        sirfsoc_pwrc_base + SIRFSOC_PWRC_TRIGGER_EN);
+}
+
+static void sirfsoc_set_sleep_mode(u32 mode)
+{
+        u32 sleep_mode = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
+		SIRFSOC_PWRC_PDN_CTRL);
+        sleep_mode &= ~(SIRFSOC_SLEEP_MODE_MASK << 1);
+        sleep_mode |= ((mode << 1));
+        sirfsoc_rtc_iobrg_writel(sleep_mode, sirfsoc_pwrc_base +
+		SIRFSOC_PWRC_PDN_CTRL);
+}
+
+int sirfsoc_pre_suspend_power_off(void)
+{
+	u32 wakeup_entry = virt_to_phys(cpu_resume);
+
+	sirfsoc_rtc_iobrg_writel(wakeup_entry,
+		SIRFSOC_PWRC_SCRATCH_PAD1);
+
+	sirfsoc_set_wakeup_source();
+
+	sirfsoc_set_sleep_mode(SIRFSOC_DEEP_SLEEP_MODE);
+
+	return 0;
+}
+
+static void sirfsoc_save_register(u32 *ptr)
+{
+	/* todo: save necessary system registers here */
+}
+
+static void sirfsoc_restore_regs(u32 *ptr)
+{
+	/* todo: restore saved system registers here */
+}
+
+static int sirfsoc_pm_enter(suspend_state_t state)
+{
+	u32 *saved_regs;
+
+	switch (state) {
+	case PM_SUSPEND_MEM:
+		saved_regs = kmalloc(1024, GFP_ATOMIC);
+		if (!saved_regs)
+			return -ENOMEM;
+
+		sirfsoc_save_register(saved_regs);
+
+		/* go zzz */
+		cpu_suspend(0, sirfsoc_finish_suspend);
+
+		sirfsoc_restore_regs(saved_regs);
+		kfree(saved_regs);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static const struct platform_suspend_ops sirfsoc_pm_ops = {
+	.enter = sirfsoc_pm_enter,
+	.valid = suspend_valid_only_mem,
+};
+
+static int __init sirfsoc_pm_init(void)
+{
+	suspend_set_ops(&sirfsoc_pm_ops);
+	return 0;
+}
+late_initcall(sirfsoc_pm_init);
+
+static struct of_device_id pwrc_ids[] = {
+	{ .compatible = "sirf,prima2-pwrc" },
+};
+
+static int __init sirfsoc_of_pwrc_init(void)
+{
+	struct device_node *np;
+	const __be32    *addrp;
+
+	np = of_find_matching_node(NULL, pwrc_ids);
+	if (!np)
+		panic("unable to find compatible pwrc node in dtb\n");
+
+	addrp = of_get_property(np, "reg", NULL);
+	if (!addrp)
+		panic("unable to find base address of pwrc node in dtb\n");
+
+	sirfsoc_pwrc_base = be32_to_cpup(addrp);
+
+	of_node_put(np);
+
+	return 0;
+}
+postcore_initcall(sirfsoc_of_pwrc_init);
+
+static struct of_device_id memc_ids[] = {
+	{ .compatible = "sirf,prima2-memc" },
+};
+
+static int __devinit sirfsoc_memc_probe(struct platform_device *op)
+{
+	struct device_node *np = op->dev.of_node;
+
+	sirfsoc_memc_base = of_iomap(np, 0);
+	if (!sirfsoc_memc_base)
+		panic("unable to map memc registers\n");
+
+	return 0;
+}
+
+static struct platform_driver sirfsoc_memc_driver = {
+	.probe		= sirfsoc_memc_probe,
+	.driver = {
+		.name = "sirfsoc-memc",
+		.owner = THIS_MODULE,
+		.of_match_table	= memc_ids,
+	},
+};
+
+static int __init sirfsoc_memc_init(void)
+{
+	return platform_driver_register(&sirfsoc_memc_driver);
+}
+postcore_initcall(sirfsoc_memc_init);
diff --git a/arch/arm/mach-prima2/pm.h b/arch/arm/mach-prima2/pm.h
new file mode 100644
index 0000000..fe2028b
--- /dev/null
+++ b/arch/arm/mach-prima2/pm.h
@@ -0,0 +1,31 @@
+/*
+ * arch/arm/mach-prima2/pm.h
+ *
+ * Copyright (C) 2011 CSR
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef _MACH_PRIMA2_PM_H_
+#define _MACH_PRIMA2_PM_H_
+
+#define SIRFSOC_PWR_SLEEPFORCE		0x01
+
+#define SIRFSOC_SLEEP_MODE_MASK         0x3
+#define SIRFSOC_DEEP_SLEEP_MODE         0x1
+
+#define SIRFSOC_PWRC_PDN_CTRL           0x0
+#define SIRFSOC_PWRC_PON_OFF            0x4
+#define SIRFSOC_PWRC_TRIGGER_EN         0x8
+#define SIRFSOC_PWRC_PIN_STATUS         0x14
+#define SIRFSOC_PWRC_SCRATCH_PAD1       0x18
+#define SIRFSOC_PWRC_SCRATCH_PAD2       0x1C
+
+#ifndef __ASSEMBLY__
+extern int sirfsoc_finish_suspend(unsigned long);
+#endif
+
+#endif
+
diff --git a/arch/arm/mach-prima2/sleep.S b/arch/arm/mach-prima2/sleep.S
new file mode 100644
index 0000000..07f8067
--- /dev/null
+++ b/arch/arm/mach-prima2/sleep.S
@@ -0,0 +1,61 @@
+/*
+ * sleep mode for CSR SiRFprimaII
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/linkage.h>
+#include <asm/ptrace.h>
+#include <asm/assembler.h>
+
+#include "pm.h"
+
+#define DENALI_CTL_22_OFF	0x58
+#define DENALI_CTL_112_OFF	0x1c0
+
+	.text
+
+ENTRY(sirfsoc_finish_suspend)
+	@ r5: 	mem controller
+	ldr     r0, =sirfsoc_memc_base
+	ldr	r5, [r0]
+	@ r7: 	rtc iobrg controller
+	ldr     r0, =sirfsoc_rtciobrg_base
+	ldr	r7, [r0]
+
+	@ Read the power control register and set the
+	@ sleep force bit.
+	ldr	r0, =SIRFSOC_PWRC_PDN_CTRL
+	bl	__sirfsoc_rtc_iobrg_readl
+	orr	r0,r0,#SIRFSOC_PWR_SLEEPFORCE
+	ldr	r1, =SIRFSOC_PWRC_PDN_CTRL
+	bl	sirfsoc_rtc_iobrg_pre_writel
+	mov	r1, #0x1
+
+	@ read the MEM ctl register and set the self
+	@ refresh bit
+
+	ldr	r2, [r5, #DENALI_CTL_22_OFF]
+	orr	r2, r2, #0x1
+
+	@ Following code has to run from cache since
+	@ the RAM is going to self refresh mode
+	.align 5
+	str	r2, [r5, #DENALI_CTL_22_OFF]
+
+1:
+	ldr	r4, [r5, #DENALI_CTL_112_OFF]
+	tst	r4, #0x1
+	bne	1b
+
+	@ write SLEEPFORCE through rtc iobridge
+
+	str	r1, [r7]
+	@ wait rtc io bridge sync
+1:
+	ldr	r3, [r7]
+	tst	r3, #0x01
+	bne	1b
+	b .
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 3/4] ARM: L2X0: move l2x0_init out of .init section
  2011-08-23  6:15 [PATCH v2 0/4] ARM: CSR: add rtciobrg and PM support Barry Song
  2011-08-23  6:15 ` [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII Barry Song
  2011-08-23  6:15 ` [PATCH v2 2/4] ARM: CSR: add PM sleep entry " Barry Song
@ 2011-08-23  6:15 ` Barry Song
  2011-08-23  6:15 ` [PATCH v2 4/4] ARM: CSR: add PM suspend/resume entries for SiRFprimaII L2 cache Barry Song
  3 siblings, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-23  6:15 UTC (permalink / raw)
  To: linux-arm-kernel

After resuming, system need to re-init L2

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 arch/arm/include/asm/hardware/cache-l2x0.h |    2 +-
 arch/arm/mm/cache-l2x0.c                   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 16bd480..407c849 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -73,7 +73,7 @@
 #define L2X0_AUX_CTRL_EARLY_BRESP_SHIFT		30
 
 #ifndef __ASSEMBLY__
-extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
+extern void l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
 #endif
 
 #endif
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 44c0867..5891ea0 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -277,7 +277,7 @@ static void l2x0_disable(void)
 	spin_unlock_irqrestore(&l2x0_lock, flags);
 }
 
-void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
+void l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
 {
 	__u32 aux;
 	__u32 cache_id;
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 4/4] ARM: CSR: add PM suspend/resume entries for SiRFprimaII L2 cache
  2011-08-23  6:15 [PATCH v2 0/4] ARM: CSR: add rtciobrg and PM support Barry Song
                   ` (2 preceding siblings ...)
  2011-08-23  6:15 ` [PATCH v2 3/4] ARM: L2X0: move l2x0_init out of .init section Barry Song
@ 2011-08-23  6:15 ` Barry Song
  3 siblings, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-23  6:15 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rongjun Ying <rongjun.ying@csr.com>

Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 arch/arm/mach-prima2/l2x0.c |   82 +++++++++++++++++++++++++++++++-----------
 1 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-prima2/l2x0.c b/arch/arm/mach-prima2/l2x0.c
index 9cda205..620c586 100644
--- a/arch/arm/mach-prima2/l2x0.c
+++ b/arch/arm/mach-prima2/l2x0.c
@@ -18,42 +18,80 @@
 #define L2X0_ADDR_FILTERING_START       0xC00
 #define L2X0_ADDR_FILTERING_END         0xC04
 
-static struct of_device_id l2x_ids[]  = {
+static void __iomem *sirfsoc_l2x0_base;
+
+static struct of_device_id l2x0_ids[]  = {
 	{ .compatible = "arm,pl310-cache" },
 };
 
-static int __init sirfsoc_of_l2x_init(void)
+static void sirfsoc_l2x0_init(void)
 {
-	struct device_node *np;
-	void __iomem *sirfsoc_l2x_base;
-
-	np = of_find_matching_node(NULL, l2x_ids);
-	if (!np)
-		panic("unable to find compatible l2x node in dtb\n");
-
-	sirfsoc_l2x_base = of_iomap(np, 0);
-	if (!sirfsoc_l2x_base)
-		panic("unable to map l2x cpu registers\n");
-
-	of_node_put(np);
-
-	if (!(readl_relaxed(sirfsoc_l2x_base + L2X0_CTRL) & 1)) {
+	if (!(readl_relaxed(sirfsoc_l2x0_base + L2X0_CTRL) & 1)) {
 		/*
 		 * set the physical memory windows L2 cache will cover
 		 */
 		writel_relaxed(PLAT_PHYS_OFFSET + 1024 * 1024 * 1024,
-			sirfsoc_l2x_base + L2X0_ADDR_FILTERING_END);
+			sirfsoc_l2x0_base + L2X0_ADDR_FILTERING_END);
 		writel_relaxed(PLAT_PHYS_OFFSET | 0x1,
-			sirfsoc_l2x_base + L2X0_ADDR_FILTERING_START);
+			sirfsoc_l2x0_base + L2X0_ADDR_FILTERING_START);
 
 		writel_relaxed(0,
-			sirfsoc_l2x_base + L2X0_TAG_LATENCY_CTRL);
+			sirfsoc_l2x0_base + L2X0_TAG_LATENCY_CTRL);
 		writel_relaxed(0,
-			sirfsoc_l2x_base + L2X0_DATA_LATENCY_CTRL);
+			sirfsoc_l2x0_base + L2X0_DATA_LATENCY_CTRL);
 	}
-	l2x0_init((void __iomem *)sirfsoc_l2x_base, 0x00040000,
+	l2x0_init((void __iomem *)sirfsoc_l2x0_base, 0x00040000,
 		0x00000000);
+}
+
+static int __init sirfsoc_of_l2x0_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_matching_node(NULL, l2x0_ids);
+	if (!np)
+		panic("unable to find compatible l2x node in dtb\n");
+
+	sirfsoc_l2x0_base = of_iomap(np, 0);
+	if (!sirfsoc_l2x0_base)
+		panic("unable to map l2x cpu registers\n");
 
+	of_node_put(np);
+
+	sirfsoc_l2x0_init();
+
+	return 0;
+}
+early_initcall(sirfsoc_of_l2x0_init);
+
+#ifdef CONFIG_PM
+#include <linux/syscore_ops.h>
+
+static int sirfsoc_l2x0_pm_suspend(void)
+{
+	writel_relaxed(0xFF, sirfsoc_l2x0_base + L2X0_CLEAN_INV_WAY);
+	while (readl_relaxed(sirfsoc_l2x0_base + L2X0_CLEAN_INV_WAY))
+		continue;
+	writel_relaxed(0x0, sirfsoc_l2x0_base + L2X0_CACHE_SYNC);
+	writel_relaxed(0x0, sirfsoc_l2x0_base + L2X0_CTRL);
+	return 0;
+}
+
+static void sirfsoc_l2x0_pm_resume(void)
+{
+	sirfsoc_l2x0_init();
+}
+
+static struct syscore_ops sirfsoc_l2x0_pm_syscore_ops = {
+	.suspend	= sirfsoc_l2x0_pm_suspend,
+	.resume		= sirfsoc_l2x0_pm_resume,
+};
+
+static int sirfsoc_l2x0_pm_init(void)
+{
+	register_syscore_ops(&sirfsoc_l2x0_pm_syscore_ops);
 	return 0;
 }
-early_initcall(sirfsoc_of_l2x_init);
+late_initcall(sirfsoc_l2x0_pm_init);
+
+#endif
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23  6:15 ` [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII Barry Song
@ 2011-08-23  8:48   ` Jamie Iles
  2011-08-23  9:33     ` Barry Song
  2011-08-23 16:01   ` Arnd Bergmann
  1 sibling, 1 reply; 25+ messages in thread
From: Jamie Iles @ 2011-08-23  8:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Barry,

On Mon, Aug 22, 2011 at 11:15:49PM -0700, Barry Song wrote:
> From: Zhiwu Song <zhiwu.song@csr.com>
> 
> The module is a bridge between the RTC clock domain and the CPU interface
> clock domain. ARM access the register of SYSRTC, GPSRTC and PWRC through
> this module.
> 
> Signed-off-by: Zhiwu Song <zhiwu.song@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

A couple of minor nits inline (sorry if I didn't pick them up last 
time).  Feel free to add a:

	Reviewed-by: Jamie Iles <jamie@jamieiles.com>

if you choose to make those changes.

Jamie

> ---
>  -v2:
>  handle feedback from Jamie Iles <jamie@jamieiles.com>
>  make rtciobrg become a device driver
>  make devices behind rtciobrg not be compatible with simple-bus
>  replace EXPORT_SYMBOL by EXPORT_SYMBOL_GPL
> 
>  arch/arm/boot/dts/prima2-cb.dts      |    2 +-
>  arch/arm/mach-prima2/Makefile        |    1 +
>  arch/arm/mach-prima2/rtciobrg.c      |  136 ++++++++++++++++++++++++++++++++++
>  include/linux/rtc/sirfsoc_rtciobrg.h |   18 +++++
>  4 files changed, 156 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/mach-prima2/rtciobrg.c
>  create mode 100644 include/linux/rtc/sirfsoc_rtciobrg.h
> 
> diff --git a/arch/arm/boot/dts/prima2-cb.dts b/arch/arm/boot/dts/prima2-cb.dts
> index 6fecc88..e5e9bc6 100644
> --- a/arch/arm/boot/dts/prima2-cb.dts
> +++ b/arch/arm/boot/dts/prima2-cb.dts
> @@ -358,7 +358,7 @@
>  		};
>  
>  		rtc-iobg {
> -			compatible = "sirf,prima2-rtciobg", "simple-bus";
> +			compatible = "sirf,prima2-rtciobg", "sirf-prima2-rtciobg-bus";
>  			#address-cells = <1>;
>  			#size-cells = <1>;
>  			reg = <0x80030000 0x10000>;
> diff --git a/arch/arm/mach-prima2/Makefile b/arch/arm/mach-prima2/Makefile
> index 7af7fc0..f49d70b 100644
> --- a/arch/arm/mach-prima2/Makefile
> +++ b/arch/arm/mach-prima2/Makefile
> @@ -3,5 +3,6 @@ obj-y += irq.o
>  obj-y += clock.o
>  obj-y += rstc.o
>  obj-y += prima2.o
> +obj-y += rtciobrg.o
>  obj-$(CONFIG_DEBUG_LL) += lluart.o
>  obj-$(CONFIG_CACHE_L2X0) += l2x0.o
> diff --git a/arch/arm/mach-prima2/rtciobrg.c b/arch/arm/mach-prima2/rtciobrg.c
> new file mode 100644
> index 0000000..1163e2c
> --- /dev/null
> +++ b/arch/arm/mach-prima2/rtciobrg.c
> @@ -0,0 +1,136 @@
> +/*
> + * RTC I/O Bridge interfaces for CSR SiRFprimaII
> + * ARM access the registers of SYSRTC, GPSRTC and PWRC through this module
> + *
> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +
> +#define SIRFSOC_CPUIOBRG_CTRL           0x00
> +#define SIRFSOC_CPUIOBRG_WRBE           0x04
> +#define SIRFSOC_CPUIOBRG_ADDR           0x08
> +#define SIRFSOC_CPUIOBRG_DATA           0x0c
> +
> +void __iomem *sirfsoc_rtciobrg_base;
> +static DEFINE_SPINLOCK(rtciobrg_lock);
> +
> +void sirfsoc_rtc_iobrg_wait_sync(void)
> +{
> +	while (readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL))
> +		continue;

It might be worth using cpu_relax() here as it includes a memory barrier 
that's what many busy wait loops do.

> +}
> +
> +void sirfsoc_rtc_iobrg_besyncing(void)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&rtciobrg_lock, flags);
> +
> +	sirfsoc_rtc_iobrg_wait_sync();
> +
> +	spin_unlock_irqrestore(&rtciobrg_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_besyncing);
> +
> +u32 __sirfsoc_rtc_iobrg_readl(u32 addr)
> +{
> +	unsigned long val;
> +
> +	sirfsoc_rtc_iobrg_wait_sync();
> +
> +	writel_relaxed(0x00, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_WRBE);
> +	writel_relaxed(addr, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_ADDR);
> +	writel_relaxed(0x01, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL);
> +
> +	sirfsoc_rtc_iobrg_wait_sync();
> +
> +	val = readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_DATA);
> +
> +	return val;

I don't think this needs the temporary 'val', but that's not a big 
problem.

> +}
> +
> +u32 sirfsoc_rtc_iobrg_readl(u32 addr)
> +{
> +	unsigned long flags, val;
> +
> +	spin_lock_irqsave(&rtciobrg_lock, flags);
> +
> +	val = __sirfsoc_rtc_iobrg_readl(addr);
> +
> +	spin_unlock_irqrestore(&rtciobrg_lock, flags);
> +
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_readl);
> +
> +void sirfsoc_rtc_iobrg_pre_writel(u32 val, u32 addr)
> +{
> +	sirfsoc_rtc_iobrg_wait_sync();
> +
> +	writel_relaxed(0xf1, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_WRBE);
> +	writel_relaxed(addr, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_ADDR);
> +
> +	writel_relaxed(val, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_DATA);
> +}
> +
> +void sirfsoc_rtc_iobrg_writel(u32 val, u32 addr)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&rtciobrg_lock, flags);
> +
> +	sirfsoc_rtc_iobrg_pre_writel(val, addr);
> +
> +	writel_relaxed(0x01, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL);
> +
> +	sirfsoc_rtc_iobrg_wait_sync();
> +
> +	spin_unlock_irqrestore(&rtciobrg_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_writel);
> +
> +static struct of_device_id rtciobrg_ids[] = {

const?

> +	{ .compatible = "sirf,prima2-rtciobg" },
> +	{}
> +};
> +
> +static int __devinit sirfsoc_rtciobrg_probe(struct platform_device *op)
> +{
> +	struct device_node *np = op->dev.of_node;
> +
> +	sirfsoc_rtciobrg_base = of_iomap(np, 0);
> +	if (!sirfsoc_rtciobrg_base)
> +		panic("unable to map rtc iobrg registers\n");
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sirfsoc_rtciobrg_driver = {
> +	.probe		= sirfsoc_rtciobrg_probe,
> +	.driver = {
> +		.name = "sirfsoc-rtciobrg",
> +		.owner = THIS_MODULE,
> +		.of_match_table	= rtciobrg_ids,
> +	},
> +};
> +
> +static int __init sirfsoc_rtciobrg_init(void)
> +{
> +	return platform_driver_register(&sirfsoc_rtciobrg_driver);
> +}
> +postcore_initcall(sirfsoc_rtciobrg_init);
> +
> +MODULE_AUTHOR("Zhiwu Song <zhiwu.song@csr.com>, "
> +		"Barry Song <baohua.song@csr.com>");
> +MODULE_DESCRIPTION("CSR SiRFprimaII rtc io bridge");
> +MODULE_LICENSE("GPL");
> +
> diff --git a/include/linux/rtc/sirfsoc_rtciobrg.h b/include/linux/rtc/sirfsoc_rtciobrg.h
> new file mode 100644
> index 0000000..2c92e1c
> --- /dev/null
> +++ b/include/linux/rtc/sirfsoc_rtciobrg.h
> @@ -0,0 +1,18 @@
> +/*
> + * RTC I/O Bridge interfaces for CSR SiRFprimaII
> + * ARM access the registers of SYSRTC, GPSRTC and PWRC through this module
> + *
> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
> + *
> + * Licensed under GPLv2 or later.
> + */
> +#ifndef _SIRFSOC_RTC_IOBRG_H_
> +#define _SIRFSOC_RTC_IOBRG_H_
> +
> +extern void sirfsoc_rtc_iobrg_besyncing(void);
> +
> +extern u32 sirfsoc_rtc_iobrg_readl(u32 addr);
> +
> +extern void sirfsoc_rtc_iobrg_writel(u32 val, u32 addr);
> +
> +#endif
> -- 
> 1.7.1
> 
> 
> 
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
> More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-23  6:15 ` [PATCH v2 2/4] ARM: CSR: add PM sleep entry " Barry Song
@ 2011-08-23  8:50   ` Jamie Iles
  2011-08-23  9:08     ` Barry Song
  2011-08-23  9:41   ` Russell King - ARM Linux
  2011-08-25  7:27   ` Shawn Guo
  2 siblings, 1 reply; 25+ messages in thread
From: Jamie Iles @ 2011-08-23  8:50 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Barry, Rongjun,

A couple of minor nits inline.

Jamie

On Mon, Aug 22, 2011 at 11:15:50PM -0700, Barry Song wrote:
> From: Rongjun Ying <rongjun.ying@csr.com>
> 
> Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  -v2:
>  delete redundant ARM mode registers save/restore
>  use generic cpu suspend
>  move l2 cache suspend/resume codes out
>  make memc become a device driver
> 
>  arch/arm/mach-prima2/pm.c    |  158 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-prima2/pm.h    |   31 ++++++++
>  arch/arm/mach-prima2/sleep.S |   61 ++++++++++++++++
>  3 files changed, 250 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-prima2/pm.c
>  create mode 100644 arch/arm/mach-prima2/pm.h
>  create mode 100644 arch/arm/mach-prima2/sleep.S
> 
> diff --git a/arch/arm/mach-prima2/pm.c b/arch/arm/mach-prima2/pm.c
> new file mode 100644
> index 0000000..75167df
> --- /dev/null
> +++ b/arch/arm/mach-prima2/pm.c
> @@ -0,0 +1,158 @@
> +/*
> + * power management entry for CSR SiRFprimaII
> + *
> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/suspend.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/io.h>
> +#include <linux/rtc/sirfsoc_rtciobrg.h>
> +#include <asm/suspend.h>
> +
> +#include "pm.h"
> +
> +static u32 sirfsoc_pwrc_base;
> +void __iomem *sirfsoc_memc_base;
> +
> +static void sirfsoc_set_wakeup_source(void)
> +{
> +        u32 pwr_trigger_en_reg;
> +        pwr_trigger_en_reg = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
> +		SIRFSOC_PWRC_TRIGGER_EN);
> +#define X_ON_KEY_B (1 << 0)
> +        sirfsoc_rtc_iobrg_writel(pwr_trigger_en_reg | X_ON_KEY_B,
> +                        sirfsoc_pwrc_base + SIRFSOC_PWRC_TRIGGER_EN);
> +}
> +
> +static void sirfsoc_set_sleep_mode(u32 mode)
> +{
> +        u32 sleep_mode = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
> +		SIRFSOC_PWRC_PDN_CTRL);
> +        sleep_mode &= ~(SIRFSOC_SLEEP_MODE_MASK << 1);
> +        sleep_mode |= ((mode << 1));
> +        sirfsoc_rtc_iobrg_writel(sleep_mode, sirfsoc_pwrc_base +
> +		SIRFSOC_PWRC_PDN_CTRL);
> +}
> +
> +int sirfsoc_pre_suspend_power_off(void)
> +{
> +	u32 wakeup_entry = virt_to_phys(cpu_resume);
> +
> +	sirfsoc_rtc_iobrg_writel(wakeup_entry,
> +		SIRFSOC_PWRC_SCRATCH_PAD1);
> +
> +	sirfsoc_set_wakeup_source();
> +
> +	sirfsoc_set_sleep_mode(SIRFSOC_DEEP_SLEEP_MODE);
> +
> +	return 0;
> +}
> +
> +static void sirfsoc_save_register(u32 *ptr)
> +{
> +	/* todo: save necessary system registers here */
> +}
> +
> +static void sirfsoc_restore_regs(u32 *ptr)
> +{
> +	/* todo: restore saved system registers here */
> +}
> +
> +static int sirfsoc_pm_enter(suspend_state_t state)
> +{
> +	u32 *saved_regs;
> +
> +	switch (state) {
> +	case PM_SUSPEND_MEM:
> +		saved_regs = kmalloc(1024, GFP_ATOMIC);
> +		if (!saved_regs)
> +			return -ENOMEM;
> +
> +		sirfsoc_save_register(saved_regs);
> +
> +		/* go zzz */
> +		cpu_suspend(0, sirfsoc_finish_suspend);
> +
> +		sirfsoc_restore_regs(saved_regs);
> +		kfree(saved_regs);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static const struct platform_suspend_ops sirfsoc_pm_ops = {
> +	.enter = sirfsoc_pm_enter,
> +	.valid = suspend_valid_only_mem,
> +};
> +
> +static int __init sirfsoc_pm_init(void)
> +{
> +	suspend_set_ops(&sirfsoc_pm_ops);
> +	return 0;
> +}
> +late_initcall(sirfsoc_pm_init);
> +
> +static struct of_device_id pwrc_ids[] = {
> +	{ .compatible = "sirf,prima2-pwrc" },

Missing a sentinel terminator and const?

> +};
> +
> +static int __init sirfsoc_of_pwrc_init(void)
> +{
> +	struct device_node *np;
> +	const __be32    *addrp;
> +
> +	np = of_find_matching_node(NULL, pwrc_ids);
> +	if (!np)
> +		panic("unable to find compatible pwrc node in dtb\n");
> +
> +	addrp = of_get_property(np, "reg", NULL);
> +	if (!addrp)
> +		panic("unable to find base address of pwrc node in dtb\n");
> +
> +	sirfsoc_pwrc_base = be32_to_cpup(addrp);
> +
> +	of_node_put(np);
> +
> +	return 0;
> +}
> +postcore_initcall(sirfsoc_of_pwrc_init);
> +
> +static struct of_device_id memc_ids[] = {
> +	{ .compatible = "sirf,prima2-memc" },

and here?

> +};
> +
> +static int __devinit sirfsoc_memc_probe(struct platform_device *op)
> +{
> +	struct device_node *np = op->dev.of_node;
> +
> +	sirfsoc_memc_base = of_iomap(np, 0);
> +	if (!sirfsoc_memc_base)
> +		panic("unable to map memc registers\n");
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sirfsoc_memc_driver = {
> +	.probe		= sirfsoc_memc_probe,
> +	.driver = {
> +		.name = "sirfsoc-memc",
> +		.owner = THIS_MODULE,
> +		.of_match_table	= memc_ids,
> +	},
> +};
> +
> +static int __init sirfsoc_memc_init(void)
> +{
> +	return platform_driver_register(&sirfsoc_memc_driver);
> +}
> +postcore_initcall(sirfsoc_memc_init);
> diff --git a/arch/arm/mach-prima2/pm.h b/arch/arm/mach-prima2/pm.h
> new file mode 100644
> index 0000000..fe2028b
> --- /dev/null
> +++ b/arch/arm/mach-prima2/pm.h
> @@ -0,0 +1,31 @@
> +/*
> + * arch/arm/mach-prima2/pm.h
> + *
> + * Copyright (C) 2011 CSR
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#ifndef _MACH_PRIMA2_PM_H_
> +#define _MACH_PRIMA2_PM_H_
> +
> +#define SIRFSOC_PWR_SLEEPFORCE		0x01
> +
> +#define SIRFSOC_SLEEP_MODE_MASK         0x3
> +#define SIRFSOC_DEEP_SLEEP_MODE         0x1
> +
> +#define SIRFSOC_PWRC_PDN_CTRL           0x0
> +#define SIRFSOC_PWRC_PON_OFF            0x4
> +#define SIRFSOC_PWRC_TRIGGER_EN         0x8
> +#define SIRFSOC_PWRC_PIN_STATUS         0x14
> +#define SIRFSOC_PWRC_SCRATCH_PAD1       0x18
> +#define SIRFSOC_PWRC_SCRATCH_PAD2       0x1C
> +
> +#ifndef __ASSEMBLY__
> +extern int sirfsoc_finish_suspend(unsigned long);
> +#endif
> +
> +#endif
> +
> diff --git a/arch/arm/mach-prima2/sleep.S b/arch/arm/mach-prima2/sleep.S
> new file mode 100644
> index 0000000..07f8067
> --- /dev/null
> +++ b/arch/arm/mach-prima2/sleep.S
> @@ -0,0 +1,61 @@
> +/*
> + * sleep mode for CSR SiRFprimaII
> + *
> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/ptrace.h>
> +#include <asm/assembler.h>
> +
> +#include "pm.h"
> +
> +#define DENALI_CTL_22_OFF	0x58
> +#define DENALI_CTL_112_OFF	0x1c0
> +
> +	.text
> +
> +ENTRY(sirfsoc_finish_suspend)
> +	@ r5: 	mem controller
> +	ldr     r0, =sirfsoc_memc_base
> +	ldr	r5, [r0]
> +	@ r7: 	rtc iobrg controller
> +	ldr     r0, =sirfsoc_rtciobrg_base
> +	ldr	r7, [r0]
> +
> +	@ Read the power control register and set the
> +	@ sleep force bit.
> +	ldr	r0, =SIRFSOC_PWRC_PDN_CTRL
> +	bl	__sirfsoc_rtc_iobrg_readl
> +	orr	r0,r0,#SIRFSOC_PWR_SLEEPFORCE
> +	ldr	r1, =SIRFSOC_PWRC_PDN_CTRL
> +	bl	sirfsoc_rtc_iobrg_pre_writel
> +	mov	r1, #0x1
> +
> +	@ read the MEM ctl register and set the self
> +	@ refresh bit
> +
> +	ldr	r2, [r5, #DENALI_CTL_22_OFF]
> +	orr	r2, r2, #0x1
> +
> +	@ Following code has to run from cache since
> +	@ the RAM is going to self refresh mode
> +	.align 5
> +	str	r2, [r5, #DENALI_CTL_22_OFF]
> +
> +1:
> +	ldr	r4, [r5, #DENALI_CTL_112_OFF]
> +	tst	r4, #0x1
> +	bne	1b
> +
> +	@ write SLEEPFORCE through rtc iobridge
> +
> +	str	r1, [r7]
> +	@ wait rtc io bridge sync
> +1:
> +	ldr	r3, [r7]
> +	tst	r3, #0x01
> +	bne	1b
> +	b .
> -- 
> 1.7.1
> 
> 
> 
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
> More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-23  8:50   ` Jamie Iles
@ 2011-08-23  9:08     ` Barry Song
  0 siblings, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-23  9:08 UTC (permalink / raw)
  To: linux-arm-kernel

>> +
>> +static struct of_device_id pwrc_ids[] = {
>> + ? ? { .compatible = "sirf,prima2-pwrc" },
>
> Missing a sentinel terminator and const?

i have agreed with you about that before, but forget to fix that yet....
Thanks for pointing out again.

-barry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23  8:48   ` Jamie Iles
@ 2011-08-23  9:33     ` Barry Song
  2011-08-23  9:53       ` Jamie Iles
  0 siblings, 1 reply; 25+ messages in thread
From: Barry Song @ 2011-08-23  9:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jamie,
Thanks.

2011/8/23 Jamie Iles <jamie@jamieiles.com>:
> Hi Barry,
>
> On Mon, Aug 22, 2011 at 11:15:49PM -0700, Barry Song wrote:
>> From: Zhiwu Song <zhiwu.song@csr.com>
>>
>> The module is a bridge between the RTC clock domain and the CPU interface
>> clock domain. ARM access the register of SYSRTC, GPSRTC and PWRC through
>> this module.
>>
>> Signed-off-by: Zhiwu Song <zhiwu.song@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>
> A couple of minor nits inline (sorry if I didn't pick them up last
> time). ?Feel free to add a:
>
> ? ? ? ?Reviewed-by: Jamie Iles <jamie@jamieiles.com>
>
> if you choose to make those changes.

yes, i will.

>
> Jamie
>
>> ---
>> ?-v2:
>> ?handle feedback from Jamie Iles <jamie@jamieiles.com>
>> ?make rtciobrg become a device driver
>> ?make devices behind rtciobrg not be compatible with simple-bus
>> ?replace EXPORT_SYMBOL by EXPORT_SYMBOL_GPL
>>
>> ?arch/arm/boot/dts/prima2-cb.dts ? ? ?| ? ?2 +-
>> ?arch/arm/mach-prima2/Makefile ? ? ? ?| ? ?1 +
>> ?arch/arm/mach-prima2/rtciobrg.c ? ? ?| ?136 ++++++++++++++++++++++++++++++++++
>> ?include/linux/rtc/sirfsoc_rtciobrg.h | ? 18 +++++
>> ?4 files changed, 156 insertions(+), 1 deletions(-)
>> ?create mode 100644 arch/arm/mach-prima2/rtciobrg.c
>> ?create mode 100644 include/linux/rtc/sirfsoc_rtciobrg.h
>>
>> diff --git a/arch/arm/boot/dts/prima2-cb.dts b/arch/arm/boot/dts/prima2-cb.dts
>> index 6fecc88..e5e9bc6 100644
>> --- a/arch/arm/boot/dts/prima2-cb.dts
>> +++ b/arch/arm/boot/dts/prima2-cb.dts
>> @@ -358,7 +358,7 @@
>> ? ? ? ? ? ? ? };
>>
>> ? ? ? ? ? ? ? rtc-iobg {
>> - ? ? ? ? ? ? ? ? ? ? compatible = "sirf,prima2-rtciobg", "simple-bus";
>> + ? ? ? ? ? ? ? ? ? ? compatible = "sirf,prima2-rtciobg", "sirf-prima2-rtciobg-bus";
>> ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>;
>> ? ? ? ? ? ? ? ? ? ? ? #size-cells = <1>;
>> ? ? ? ? ? ? ? ? ? ? ? reg = <0x80030000 0x10000>;
>> diff --git a/arch/arm/mach-prima2/Makefile b/arch/arm/mach-prima2/Makefile
>> index 7af7fc0..f49d70b 100644
>> --- a/arch/arm/mach-prima2/Makefile
>> +++ b/arch/arm/mach-prima2/Makefile
>> @@ -3,5 +3,6 @@ obj-y += irq.o
>> ?obj-y += clock.o
>> ?obj-y += rstc.o
>> ?obj-y += prima2.o
>> +obj-y += rtciobrg.o
>> ?obj-$(CONFIG_DEBUG_LL) += lluart.o
>> ?obj-$(CONFIG_CACHE_L2X0) += l2x0.o
>> diff --git a/arch/arm/mach-prima2/rtciobrg.c b/arch/arm/mach-prima2/rtciobrg.c
>> new file mode 100644
>> index 0000000..1163e2c
>> --- /dev/null
>> +++ b/arch/arm/mach-prima2/rtciobrg.c
>> @@ -0,0 +1,136 @@
>> +/*
>> + * RTC I/O Bridge interfaces for CSR SiRFprimaII
>> + * ARM access the registers of SYSRTC, GPSRTC and PWRC through this module
>> + *
>> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
>> + *
>> + * Licensed under GPLv2 or later.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +
>> +#define SIRFSOC_CPUIOBRG_CTRL ? ? ? ? ? 0x00
>> +#define SIRFSOC_CPUIOBRG_WRBE ? ? ? ? ? 0x04
>> +#define SIRFSOC_CPUIOBRG_ADDR ? ? ? ? ? 0x08
>> +#define SIRFSOC_CPUIOBRG_DATA ? ? ? ? ? 0x0c
>> +
>> +void __iomem *sirfsoc_rtciobrg_base;
>> +static DEFINE_SPINLOCK(rtciobrg_lock);
>> +
>> +void sirfsoc_rtc_iobrg_wait_sync(void)
>> +{
>> + ? ? while (readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL))
>> + ? ? ? ? ? ? continue;
>
> It might be worth using cpu_relax() here as it includes a memory barrier
> that's what many busy wait loops do.

yes. generically people add a cpu_relax() in busy wait and that makes
lots of senses. it seems readl_relaxed() can keep the execution
sequence even without the barrier?
Then there are:

static inline void cache_wait_way(void __iomem *reg, unsigned long mask)
{
        /* wait for cache operation by line or way to complete */
        while (readl_relaxed(reg) & mask)
                ;
}

static inline void ux500_cache_wait(void __iomem *reg, unsigned long mask)
{
        /* wait for the operation to complete */
        while (readl_relaxed(reg) & mask)
                ;
}
>
>> +}
>> +
>> +void sirfsoc_rtc_iobrg_besyncing(void)
>> +{
>> + ? ? unsigned long flags;
>> +
>> + ? ? spin_lock_irqsave(&rtciobrg_lock, flags);
>> +
>> + ? ? sirfsoc_rtc_iobrg_wait_sync();
>> +
>> + ? ? spin_unlock_irqrestore(&rtciobrg_lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_besyncing);
>> +
>> +u32 __sirfsoc_rtc_iobrg_readl(u32 addr)
>> +{
>> + ? ? unsigned long val;
>> +
>> + ? ? sirfsoc_rtc_iobrg_wait_sync();
>> +
>> + ? ? writel_relaxed(0x00, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_WRBE);
>> + ? ? writel_relaxed(addr, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_ADDR);
>> + ? ? writel_relaxed(0x01, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL);
>> +
>> + ? ? sirfsoc_rtc_iobrg_wait_sync();
>> +
>> + ? ? val = readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_DATA);
>> +
>> + ? ? return val;
>
> I don't think this needs the temporary 'val', but that's not a big
> problem.

i think it can be "return readl_relaxed(sirfsoc_rtciobrg_base +
SIRFSOC_CPUIOBRG_DATA);"

>
>> +}
>> +
>> +u32 sirfsoc_rtc_iobrg_readl(u32 addr)
>> +{
>> + ? ? unsigned long flags, val;
>> +
>> + ? ? spin_lock_irqsave(&rtciobrg_lock, flags);
>> +
>> + ? ? val = __sirfsoc_rtc_iobrg_readl(addr);
>> +
>> + ? ? spin_unlock_irqrestore(&rtciobrg_lock, flags);
>> +
>> + ? ? return val;
>> +}
>> +EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_readl);
>> +
>> +void sirfsoc_rtc_iobrg_pre_writel(u32 val, u32 addr)
>> +{
>> + ? ? sirfsoc_rtc_iobrg_wait_sync();
>> +
>> + ? ? writel_relaxed(0xf1, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_WRBE);
>> + ? ? writel_relaxed(addr, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_ADDR);
>> +
>> + ? ? writel_relaxed(val, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_DATA);
>> +}
>> +
>> +void sirfsoc_rtc_iobrg_writel(u32 val, u32 addr)
>> +{
>> + ? ? unsigned long flags;
>> +
>> + ? ? spin_lock_irqsave(&rtciobrg_lock, flags);
>> +
>> + ? ? sirfsoc_rtc_iobrg_pre_writel(val, addr);
>> +
>> + ? ? writel_relaxed(0x01, sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL);
>> +
>> + ? ? sirfsoc_rtc_iobrg_wait_sync();
>> +
>> + ? ? spin_unlock_irqrestore(&rtciobrg_lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(sirfsoc_rtc_iobrg_writel);
>> +
>> +static struct of_device_id rtciobrg_ids[] = {
>
> const?

true.

>
>> + ? ? { .compatible = "sirf,prima2-rtciobg" },
>> + ? ? {}
>> +};
>> +
>> +static int __devinit sirfsoc_rtciobrg_probe(struct platform_device *op)
>> +{
>> + ? ? struct device_node *np = op->dev.of_node;
>> +
>> + ? ? sirfsoc_rtciobrg_base = of_iomap(np, 0);
>> + ? ? if (!sirfsoc_rtciobrg_base)
>> + ? ? ? ? ? ? panic("unable to map rtc iobrg registers\n");
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static struct platform_driver sirfsoc_rtciobrg_driver = {
>> + ? ? .probe ? ? ? ? ?= sirfsoc_rtciobrg_probe,
>> + ? ? .driver = {
>> + ? ? ? ? ? ? .name = "sirfsoc-rtciobrg",
>> + ? ? ? ? ? ? .owner = THIS_MODULE,
>> + ? ? ? ? ? ? .of_match_table = rtciobrg_ids,
>> + ? ? },
>> +};
>> +
>> +static int __init sirfsoc_rtciobrg_init(void)
>> +{
>> + ? ? return platform_driver_register(&sirfsoc_rtciobrg_driver);
>> +}
>> +postcore_initcall(sirfsoc_rtciobrg_init);
>> +
>> +MODULE_AUTHOR("Zhiwu Song <zhiwu.song@csr.com>, "
>> + ? ? ? ? ? ? "Barry Song <baohua.song@csr.com>");
>> +MODULE_DESCRIPTION("CSR SiRFprimaII rtc io bridge");
>> +MODULE_LICENSE("GPL");
>> +
>> diff --git a/include/linux/rtc/sirfsoc_rtciobrg.h b/include/linux/rtc/sirfsoc_rtciobrg.h
>> new file mode 100644
>> index 0000000..2c92e1c
>> --- /dev/null
>> +++ b/include/linux/rtc/sirfsoc_rtciobrg.h
>> @@ -0,0 +1,18 @@
>> +/*
>> + * RTC I/O Bridge interfaces for CSR SiRFprimaII
>> + * ARM access the registers of SYSRTC, GPSRTC and PWRC through this module
>> + *
>> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
>> + *
>> + * Licensed under GPLv2 or later.
>> + */
>> +#ifndef _SIRFSOC_RTC_IOBRG_H_
>> +#define _SIRFSOC_RTC_IOBRG_H_
>> +
>> +extern void sirfsoc_rtc_iobrg_besyncing(void);
>> +
>> +extern u32 sirfsoc_rtc_iobrg_readl(u32 addr);
>> +
>> +extern void sirfsoc_rtc_iobrg_writel(u32 val, u32 addr);
>> +
>> +#endif
>> --
>> 1.7.1
-barry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-23  6:15 ` [PATCH v2 2/4] ARM: CSR: add PM sleep entry " Barry Song
  2011-08-23  8:50   ` Jamie Iles
@ 2011-08-23  9:41   ` Russell King - ARM Linux
  2011-08-27  9:53     ` Barry Song
  2011-08-25  7:27   ` Shawn Guo
  2 siblings, 1 reply; 25+ messages in thread
From: Russell King - ARM Linux @ 2011-08-23  9:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 22, 2011 at 11:15:50PM -0700, Barry Song wrote:
> From: Rongjun Ying <rongjun.ying@csr.com>
> 
> Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  -v2:
>  delete redundant ARM mode registers save/restore
>  use generic cpu suspend
>  move l2 cache suspend/resume codes out
>  make memc become a device driver

This looks much better, thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23  9:33     ` Barry Song
@ 2011-08-23  9:53       ` Jamie Iles
  2011-08-23 15:59         ` Arnd Bergmann
  0 siblings, 1 reply; 25+ messages in thread
From: Jamie Iles @ 2011-08-23  9:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 23, 2011 at 05:33:39PM +0800, Barry Song wrote:
> 2011/8/23 Jamie Iles <jamie@jamieiles.com>:
> > On Mon, Aug 22, 2011 at 11:15:49PM -0700, Barry Song wrote:
[...]
> >> +void sirfsoc_rtc_iobrg_wait_sync(void)
> >> +{
> >> + ? ? while (readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL))
> >> + ? ? ? ? ? ? continue;
> >
> > It might be worth using cpu_relax() here as it includes a memory barrier
> > that's what many busy wait loops do.
> 
> yes. generically people add a cpu_relax() in busy wait and that makes
> lots of senses. it seems readl_relaxed() can keep the execution
> sequence even without the barrier?

Well readl_relaxed() uses __raw_readl() which marks the dereference as 
volatile, so you should be okay without the cpu_relax() here.  Having 
said that, cpu_relax() is a little more explicit, so whatever you think 
most appropriate I guess.

Jamie

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23  9:53       ` Jamie Iles
@ 2011-08-23 15:59         ` Arnd Bergmann
  2011-08-24  1:45           ` Barry Song
  0 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2011-08-23 15:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 23 August 2011, Jamie Iles wrote:
> On Tue, Aug 23, 2011 at 05:33:39PM +0800, Barry Song wrote:
> > 2011/8/23 Jamie Iles <jamie@jamieiles.com>:
> > > On Mon, Aug 22, 2011 at 11:15:49PM -0700, Barry Song wrote:
> [...]
> > >> +void sirfsoc_rtc_iobrg_wait_sync(void)
> > >> +{
> > >> +     while (readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL))
> > >> +             continue;
> > >
> > > It might be worth using cpu_relax() here as it includes a memory barrier
> > > that's what many busy wait loops do.
> > 
> > yes. generically people add a cpu_relax() in busy wait and that makes
> > lots of senses. it seems readl_relaxed() can keep the execution
> > sequence even without the barrier?
> 
> Well readl_relaxed() uses __raw_readl() which marks the dereference as 
> volatile, so you should be okay without the cpu_relax() here.  Having 
> said that, cpu_relax() is a little more explicit, so whatever you think 
> most appropriate I guess.

I agree with Jamie, mostly because using cpu_relax in busy loops is
a well-known idiom in the kernel. It's more for documentation purposes
than technically needed here.

	Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23  6:15 ` [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII Barry Song
  2011-08-23  8:48   ` Jamie Iles
@ 2011-08-23 16:01   ` Arnd Bergmann
  2011-08-23 16:15     ` Arnd Bergmann
  2011-08-24  1:42     ` Barry Song
  1 sibling, 2 replies; 25+ messages in thread
From: Arnd Bergmann @ 2011-08-23 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 23 August 2011, Barry Song wrote:
> From: Zhiwu Song <zhiwu.song@csr.com>
> 
> The module is a bridge between the RTC clock domain and the CPU interface
> clock domain. ARM access the register of SYSRTC, GPSRTC and PWRC through
> this module.
> 
> Signed-off-by: Zhiwu Song <zhiwu.song@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---

Acked-by: Arnd Bergmann <arnd@arndb.de>

Looks basically good now. Aside from the trivial things that Jamie
already pointed out, there is one more that I noticed:

> +
> +void __iomem *sirfsoc_rtciobrg_base;
> +static DEFINE_SPINLOCK(rtciobrg_lock);

sirfsoc_rtciobrg_base should be static as well. No other code can
properly access it anyway since the lock is static, too.

	Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23 16:01   ` Arnd Bergmann
@ 2011-08-23 16:15     ` Arnd Bergmann
  2011-08-24  1:43       ` Barry Song
  2011-08-24  1:42     ` Barry Song
  1 sibling, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2011-08-23 16:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 23 August 2011, Arnd Bergmann wrote:
> > +
> > +void __iomem *sirfsoc_rtciobrg_base;
> > +static DEFINE_SPINLOCK(rtciobrg_lock);
> 
> sirfsoc_rtciobrg_base should be static as well. No other code can
> properly access it anyway since the lock is static, too.

Ah, I understand now how it's used from assembly code, but it took me a while
to figure out why it's done that way and that I couldn't come up with a better
solution either. Please add a comment above sirfsoc_rtciobrg_base and
sirfsoc_pwrc_base explaining that they are used from the suspend code and
that you don't need locking there.

	Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23 16:01   ` Arnd Bergmann
  2011-08-23 16:15     ` Arnd Bergmann
@ 2011-08-24  1:42     ` Barry Song
  1 sibling, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-24  1:42 UTC (permalink / raw)
  To: linux-arm-kernel

2011/8/24 Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 23 August 2011, Barry Song wrote:
>> From: Zhiwu Song <zhiwu.song@csr.com>
>>
>> The module is a bridge between the RTC clock domain and the CPU interface
>> clock domain. ARM access the register of SYSRTC, GPSRTC and PWRC through
>> this module.
>>
>> Signed-off-by: Zhiwu Song <zhiwu.song@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> Looks basically good now. Aside from the trivial things that Jamie
> already pointed out, there is one more that I noticed:
>
>> +
>> +void __iomem *sirfsoc_rtciobrg_base;
>> +static DEFINE_SPINLOCK(rtciobrg_lock);
>
> sirfsoc_rtciobrg_base should be static as well. No other code can
> properly access it anyway since the lock is static, too.

sirfsoc_rtciobrg_base is accessed by sleep.S too after DRAM enter
self-refresh and deepsleep is executed in cache.

        @ write SLEEPFORCE through rtc iobridge

        str     r1, [r7]
        @ wait rtc io bridge sync


>
> ? ? ? ?Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23 16:15     ` Arnd Bergmann
@ 2011-08-24  1:43       ` Barry Song
  0 siblings, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-24  1:43 UTC (permalink / raw)
  To: linux-arm-kernel

2011/8/24 Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 23 August 2011, Arnd Bergmann wrote:
>> > +
>> > +void __iomem *sirfsoc_rtciobrg_base;
>> > +static DEFINE_SPINLOCK(rtciobrg_lock);
>>
>> sirfsoc_rtciobrg_base should be static as well. No other code can
>> properly access it anyway since the lock is static, too.
>
> Ah, I understand now how it's used from assembly code, but it took me a while
> to figure out why it's done that way and that I couldn't come up with a better
> solution either. Please add a comment above sirfsoc_rtciobrg_base and
> sirfsoc_pwrc_base explaining that they are used from the suspend code and
> that you don't need locking there.

ok.
missed this one in my last reply.

>
> ? ? ? ?Arnd
>

-barry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII
  2011-08-23 15:59         ` Arnd Bergmann
@ 2011-08-24  1:45           ` Barry Song
  0 siblings, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-24  1:45 UTC (permalink / raw)
  To: linux-arm-kernel

2011/8/23 Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 23 August 2011, Jamie Iles wrote:
>> On Tue, Aug 23, 2011 at 05:33:39PM +0800, Barry Song wrote:
>> > 2011/8/23 Jamie Iles <jamie@jamieiles.com>:
>> > > On Mon, Aug 22, 2011 at 11:15:49PM -0700, Barry Song wrote:
>> [...]
>> > >> +void sirfsoc_rtc_iobrg_wait_sync(void)
>> > >> +{
>> > >> + ? ? while (readl_relaxed(sirfsoc_rtciobrg_base + SIRFSOC_CPUIOBRG_CTRL))
>> > >> + ? ? ? ? ? ? continue;
>> > >
>> > > It might be worth using cpu_relax() here as it includes a memory barrier
>> > > that's what many busy wait loops do.
>> >
>> > yes. generically people add a cpu_relax() in busy wait and that makes
>> > lots of senses. it seems readl_relaxed() can keep the execution
>> > sequence even without the barrier?
>>
>> Well readl_relaxed() uses __raw_readl() which marks the dereference as
>> volatile, so you should be okay without the cpu_relax() here. ?Having
>> said that, cpu_relax() is a little more explicit, so whatever you think
>> most appropriate I guess.
>
> I agree with Jamie, mostly because using cpu_relax in busy loops is
> a well-known idiom in the kernel. It's more for documentation purposes
> than technically needed here.

ok

>
> ? ? ? ?Arnd
>
-barry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-23  6:15 ` [PATCH v2 2/4] ARM: CSR: add PM sleep entry " Barry Song
  2011-08-23  8:50   ` Jamie Iles
  2011-08-23  9:41   ` Russell King - ARM Linux
@ 2011-08-25  7:27   ` Shawn Guo
  2011-08-25  7:30     ` Barry Song
  2 siblings, 1 reply; 25+ messages in thread
From: Shawn Guo @ 2011-08-25  7:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 22, 2011 at 11:15:50PM -0700, Barry Song wrote:
> From: Rongjun Ying <rongjun.ying@csr.com>
> 
> Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  -v2:
>  delete redundant ARM mode registers save/restore
>  use generic cpu suspend
>  move l2 cache suspend/resume codes out
>  make memc become a device driver
> 
>  arch/arm/mach-prima2/pm.c    |  158 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-prima2/pm.h    |   31 ++++++++
>  arch/arm/mach-prima2/sleep.S |   61 ++++++++++++++++
>  3 files changed, 250 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-prima2/pm.c
>  create mode 100644 arch/arm/mach-prima2/pm.h
>  create mode 100644 arch/arm/mach-prima2/sleep.S
> 
> diff --git a/arch/arm/mach-prima2/pm.c b/arch/arm/mach-prima2/pm.c
> new file mode 100644
> index 0000000..75167df
> --- /dev/null
> +++ b/arch/arm/mach-prima2/pm.c
> @@ -0,0 +1,158 @@
> +/*
> + * power management entry for CSR SiRFprimaII
> + *
> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/suspend.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/io.h>
> +#include <linux/rtc/sirfsoc_rtciobrg.h>
> +#include <asm/suspend.h>
> +
> +#include "pm.h"
> +
> +static u32 sirfsoc_pwrc_base;
> +void __iomem *sirfsoc_memc_base;
> +
> +static void sirfsoc_set_wakeup_source(void)
> +{
> +        u32 pwr_trigger_en_reg;
> +        pwr_trigger_en_reg = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
> +		SIRFSOC_PWRC_TRIGGER_EN);
> +#define X_ON_KEY_B (1 << 0)
> +        sirfsoc_rtc_iobrg_writel(pwr_trigger_en_reg | X_ON_KEY_B,
> +                        sirfsoc_pwrc_base + SIRFSOC_PWRC_TRIGGER_EN);
> +}
> +
> +static void sirfsoc_set_sleep_mode(u32 mode)
> +{
> +        u32 sleep_mode = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
> +		SIRFSOC_PWRC_PDN_CTRL);
> +        sleep_mode &= ~(SIRFSOC_SLEEP_MODE_MASK << 1);
> +        sleep_mode |= ((mode << 1));

Redundant parentheses.

> +        sirfsoc_rtc_iobrg_writel(sleep_mode, sirfsoc_pwrc_base +
> +		SIRFSOC_PWRC_PDN_CTRL);
> +}
> +
> +int sirfsoc_pre_suspend_power_off(void)
> +{
> +	u32 wakeup_entry = virt_to_phys(cpu_resume);
> +
> +	sirfsoc_rtc_iobrg_writel(wakeup_entry,
> +		SIRFSOC_PWRC_SCRATCH_PAD1);
> +
> +	sirfsoc_set_wakeup_source();
> +
> +	sirfsoc_set_sleep_mode(SIRFSOC_DEEP_SLEEP_MODE);
> +
> +	return 0;
> +}
> +
If I read it correctly, this function sets up resume entry, but I
failed to see it being called anywhere?  Am I missing anything?

> +static void sirfsoc_save_register(u32 *ptr)
> +{
> +	/* todo: save necessary system registers here */
> +}
> +
> +static void sirfsoc_restore_regs(u32 *ptr)
> +{
> +	/* todo: restore saved system registers here */
> +}
> +
> +static int sirfsoc_pm_enter(suspend_state_t state)
> +{
> +	u32 *saved_regs;
> +
> +	switch (state) {
> +	case PM_SUSPEND_MEM:
> +		saved_regs = kmalloc(1024, GFP_ATOMIC);
> +		if (!saved_regs)
> +			return -ENOMEM;
> +
> +		sirfsoc_save_register(saved_regs);
> +
> +		/* go zzz */
> +		cpu_suspend(0, sirfsoc_finish_suspend);
> +
> +		sirfsoc_restore_regs(saved_regs);
> +		kfree(saved_regs);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static const struct platform_suspend_ops sirfsoc_pm_ops = {
> +	.enter = sirfsoc_pm_enter,
> +	.valid = suspend_valid_only_mem,
> +};
> +
> +static int __init sirfsoc_pm_init(void)
> +{
> +	suspend_set_ops(&sirfsoc_pm_ops);
> +	return 0;
> +}
> +late_initcall(sirfsoc_pm_init);
> +

8<---
> +static struct of_device_id pwrc_ids[] = {
> +	{ .compatible = "sirf,prima2-pwrc" },
> +};
> +
> +static int __init sirfsoc_of_pwrc_init(void)
> +{
> +	struct device_node *np;
> +	const __be32    *addrp;
> +
> +	np = of_find_matching_node(NULL, pwrc_ids);
> +	if (!np)
> +		panic("unable to find compatible pwrc node in dtb\n");
> +
> +	addrp = of_get_property(np, "reg", NULL);
> +	if (!addrp)
> +		panic("unable to find base address of pwrc node in dtb\n");
> +
> +	sirfsoc_pwrc_base = be32_to_cpup(addrp);
> +
> +	of_node_put(np);
--->8

To me, it seems that the above code snippet can be as simply as the
following.

	np = of_find_compatible_node(NULL, NULL, "sirf,prima2-pwrc");
	sirfsoc_pwrc_base = of_iomap(np, 0);
	if (!sirfsoc_pwrc_base)
		return -ENOMEM;

> +
> +	return 0;
> +}
> +postcore_initcall(sirfsoc_of_pwrc_init);
> +
> +static struct of_device_id memc_ids[] = {
> +	{ .compatible = "sirf,prima2-memc" },
> +};
> +
> +static int __devinit sirfsoc_memc_probe(struct platform_device *op)
> +{
> +	struct device_node *np = op->dev.of_node;
> +
> +	sirfsoc_memc_base = of_iomap(np, 0);
> +	if (!sirfsoc_memc_base)
> +		panic("unable to map memc registers\n");
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sirfsoc_memc_driver = {
> +	.probe		= sirfsoc_memc_probe,
> +	.driver = {
> +		.name = "sirfsoc-memc",
> +		.owner = THIS_MODULE,
> +		.of_match_table	= memc_ids,
> +	},
> +};
> +
> +static int __init sirfsoc_memc_init(void)
> +{
> +	return platform_driver_register(&sirfsoc_memc_driver);
> +}
> +postcore_initcall(sirfsoc_memc_init);

Doing the same thing - mapping address, why sirfsoc_pwrc_base uses
a function, while sirfsoc_memc_base needs a platform_driver?  You
will have more stuff about memc to add there?

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-25  7:27   ` Shawn Guo
@ 2011-08-25  7:30     ` Barry Song
  2011-08-25  8:21       ` Shawn Guo
  0 siblings, 1 reply; 25+ messages in thread
From: Barry Song @ 2011-08-25  7:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Shawn,
2011/8/25 Shawn Guo <shawn.guo@freescale.com>:
> On Mon, Aug 22, 2011 at 11:15:50PM -0700, Barry Song wrote:
>> From: Rongjun Ying <rongjun.ying@csr.com>
>>
>> Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>> ?-v2:
>> ?delete redundant ARM mode registers save/restore
>> ?use generic cpu suspend
>> ?move l2 cache suspend/resume codes out
>> ?make memc become a device driver
>>
>> ?arch/arm/mach-prima2/pm.c ? ?| ?158 ++++++++++++++++++++++++++++++++++++++++++
>> ?arch/arm/mach-prima2/pm.h ? ?| ? 31 ++++++++
>> ?arch/arm/mach-prima2/sleep.S | ? 61 ++++++++++++++++
>> ?3 files changed, 250 insertions(+), 0 deletions(-)
>> ?create mode 100644 arch/arm/mach-prima2/pm.c
>> ?create mode 100644 arch/arm/mach-prima2/pm.h
>> ?create mode 100644 arch/arm/mach-prima2/sleep.S
>>
>> diff --git a/arch/arm/mach-prima2/pm.c b/arch/arm/mach-prima2/pm.c
>> new file mode 100644
>> index 0000000..75167df
>> --- /dev/null
>> +++ b/arch/arm/mach-prima2/pm.c
>> @@ -0,0 +1,158 @@
>> +/*
>> + * power management entry for CSR SiRFprimaII
>> + *
>> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
>> + *
>> + * Licensed under GPLv2 or later.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/suspend.h>
>> +#include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/io.h>
>> +#include <linux/rtc/sirfsoc_rtciobrg.h>
>> +#include <asm/suspend.h>
>> +
>> +#include "pm.h"
>> +
>> +static u32 sirfsoc_pwrc_base;
>> +void __iomem *sirfsoc_memc_base;
>> +
>> +static void sirfsoc_set_wakeup_source(void)
>> +{
>> + ? ? ? ?u32 pwr_trigger_en_reg;
>> + ? ? ? ?pwr_trigger_en_reg = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
>> + ? ? ? ? ? ? SIRFSOC_PWRC_TRIGGER_EN);
>> +#define X_ON_KEY_B (1 << 0)
>> + ? ? ? ?sirfsoc_rtc_iobrg_writel(pwr_trigger_en_reg | X_ON_KEY_B,
>> + ? ? ? ? ? ? ? ? ? ? ? ?sirfsoc_pwrc_base + SIRFSOC_PWRC_TRIGGER_EN);
>> +}
>> +
>> +static void sirfsoc_set_sleep_mode(u32 mode)
>> +{
>> + ? ? ? ?u32 sleep_mode = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
>> + ? ? ? ? ? ? SIRFSOC_PWRC_PDN_CTRL);
>> + ? ? ? ?sleep_mode &= ~(SIRFSOC_SLEEP_MODE_MASK << 1);
>> + ? ? ? ?sleep_mode |= ((mode << 1));
>
> Redundant parentheses.
>
>> + ? ? ? ?sirfsoc_rtc_iobrg_writel(sleep_mode, sirfsoc_pwrc_base +
>> + ? ? ? ? ? ? SIRFSOC_PWRC_PDN_CTRL);
>> +}
>> +
>> +int sirfsoc_pre_suspend_power_off(void)
>> +{
>> + ? ? u32 wakeup_entry = virt_to_phys(cpu_resume);
>> +
>> + ? ? sirfsoc_rtc_iobrg_writel(wakeup_entry,
>> + ? ? ? ? ? ? SIRFSOC_PWRC_SCRATCH_PAD1);
>> +
>> + ? ? sirfsoc_set_wakeup_source();
>> +
>> + ? ? sirfsoc_set_sleep_mode(SIRFSOC_DEEP_SLEEP_MODE);
>> +
>> + ? ? return 0;
>> +}
>> +
> If I read it correctly, this function sets up resume entry, but I
> failed to see it being called anywhere? ?Am I missing anything?

lose this while sending patch, it is before cpu_suspend() called.
Thanks for review.

>
>> +static void sirfsoc_save_register(u32 *ptr)
>> +{
>> + ? ? /* todo: save necessary system registers here */
>> +}
>> +
>> +static void sirfsoc_restore_regs(u32 *ptr)
>> +{
>> + ? ? /* todo: restore saved system registers here */
>> +}
>> +
>> +static int sirfsoc_pm_enter(suspend_state_t state)
>> +{
>> + ? ? u32 *saved_regs;
>> +
>> + ? ? switch (state) {
>> + ? ? case PM_SUSPEND_MEM:
>> + ? ? ? ? ? ? saved_regs = kmalloc(1024, GFP_ATOMIC);
>> + ? ? ? ? ? ? if (!saved_regs)
>> + ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
>> +
>> + ? ? ? ? ? ? sirfsoc_save_register(saved_regs);
>> +
>> + ? ? ? ? ? ? /* go zzz */
>> + ? ? ? ? ? ? cpu_suspend(0, sirfsoc_finish_suspend);
>> +
>> + ? ? ? ? ? ? sirfsoc_restore_regs(saved_regs);
>> + ? ? ? ? ? ? kfree(saved_regs);
>> + ? ? ? ? ? ? break;
>> + ? ? default:
>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? }
>> + ? ? return 0;
>> +}
>> +
>> +static const struct platform_suspend_ops sirfsoc_pm_ops = {
>> + ? ? .enter = sirfsoc_pm_enter,
>> + ? ? .valid = suspend_valid_only_mem,
>> +};
>> +
>> +static int __init sirfsoc_pm_init(void)
>> +{
>> + ? ? suspend_set_ops(&sirfsoc_pm_ops);
>> + ? ? return 0;
>> +}
>> +late_initcall(sirfsoc_pm_init);
>> +
>
> 8<---
>> +static struct of_device_id pwrc_ids[] = {
>> + ? ? { .compatible = "sirf,prima2-pwrc" },
>> +};
>> +
>> +static int __init sirfsoc_of_pwrc_init(void)
>> +{
>> + ? ? struct device_node *np;
>> + ? ? const __be32 ? ?*addrp;
>> +
>> + ? ? np = of_find_matching_node(NULL, pwrc_ids);
>> + ? ? if (!np)
>> + ? ? ? ? ? ? panic("unable to find compatible pwrc node in dtb\n");
>> +
>> + ? ? addrp = of_get_property(np, "reg", NULL);
>> + ? ? if (!addrp)
>> + ? ? ? ? ? ? panic("unable to find base address of pwrc node in dtb\n");
>> +
>> + ? ? sirfsoc_pwrc_base = be32_to_cpup(addrp);
>> +
>> + ? ? of_node_put(np);
> --->8
>
> To me, it seems that the above code snippet can be as simply as the
> following.
>
> ? ? ? ?np = of_find_compatible_node(NULL, NULL, "sirf,prima2-pwrc");
> ? ? ? ?sirfsoc_pwrc_base = of_iomap(np, 0);
> ? ? ? ?if (!sirfsoc_pwrc_base)
> ? ? ? ? ? ? ? ?return -ENOMEM;

you might want to read earlier thread in v1. PWRC is not in memory space.

>
>> +
>> + ? ? return 0;
>> +}
>> +postcore_initcall(sirfsoc_of_pwrc_init);
>> +
>> +static struct of_device_id memc_ids[] = {
>> + ? ? { .compatible = "sirf,prima2-memc" },
>> +};
>> +
>> +static int __devinit sirfsoc_memc_probe(struct platform_device *op)
>> +{
>> + ? ? struct device_node *np = op->dev.of_node;
>> +
>> + ? ? sirfsoc_memc_base = of_iomap(np, 0);
>> + ? ? if (!sirfsoc_memc_base)
>> + ? ? ? ? ? ? panic("unable to map memc registers\n");
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static struct platform_driver sirfsoc_memc_driver = {
>> + ? ? .probe ? ? ? ? ?= sirfsoc_memc_probe,
>> + ? ? .driver = {
>> + ? ? ? ? ? ? .name = "sirfsoc-memc",
>> + ? ? ? ? ? ? .owner = THIS_MODULE,
>> + ? ? ? ? ? ? .of_match_table = memc_ids,
>> + ? ? },
>> +};
>> +
>> +static int __init sirfsoc_memc_init(void)
>> +{
>> + ? ? return platform_driver_register(&sirfsoc_memc_driver);
>> +}
>> +postcore_initcall(sirfsoc_memc_init);
>
> Doing the same thing - mapping address, why sirfsoc_pwrc_base uses
> a function, while sirfsoc_memc_base needs a platform_driver? ?You
> will have more stuff about memc to add there?

memc is in memory space, actually simple_bus, then a platform device
has existed for it.
pwrc is now not compitable with simple_bus. it looks like not worth a
platform for the moment too.

>
> --
> Regards,
> Shawn

Thanks
Barry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-25  7:30     ` Barry Song
@ 2011-08-25  8:21       ` Shawn Guo
  2011-08-25  8:21         ` Barry Song
  2011-08-25 15:56         ` Arnd Bergmann
  0 siblings, 2 replies; 25+ messages in thread
From: Shawn Guo @ 2011-08-25  8:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Aug 25, 2011 at 03:30:27PM +0800, Barry Song wrote:
> Hi Shawn,
> 2011/8/25 Shawn Guo <shawn.guo@freescale.com>:
> > On Mon, Aug 22, 2011 at 11:15:50PM -0700, Barry Song wrote:
> >> From: Rongjun Ying <rongjun.ying@csr.com>
> >>
> >> Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
> >> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> >> ---
> >> ?-v2:
> >> ?delete redundant ARM mode registers save/restore
> >> ?use generic cpu suspend
> >> ?move l2 cache suspend/resume codes out
> >> ?make memc become a device driver
> >>
> >> ?arch/arm/mach-prima2/pm.c ? ?| ?158 ++++++++++++++++++++++++++++++++++++++++++
> >> ?arch/arm/mach-prima2/pm.h ? ?| ? 31 ++++++++
> >> ?arch/arm/mach-prima2/sleep.S | ? 61 ++++++++++++++++
> >> ?3 files changed, 250 insertions(+), 0 deletions(-)
> >> ?create mode 100644 arch/arm/mach-prima2/pm.c
> >> ?create mode 100644 arch/arm/mach-prima2/pm.h
> >> ?create mode 100644 arch/arm/mach-prima2/sleep.S
> >>
> >> diff --git a/arch/arm/mach-prima2/pm.c b/arch/arm/mach-prima2/pm.c
> >> new file mode 100644
> >> index 0000000..75167df
> >> --- /dev/null
> >> +++ b/arch/arm/mach-prima2/pm.c
> >> @@ -0,0 +1,158 @@
> >> +/*
> >> + * power management entry for CSR SiRFprimaII
> >> + *
> >> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
> >> + *
> >> + * Licensed under GPLv2 or later.
> >> + */
> >> +
> >> +#include <linux/kernel.h>
> >> +#include <linux/suspend.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_address.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/of_platform.h>
> >> +#include <linux/io.h>
> >> +#include <linux/rtc/sirfsoc_rtciobrg.h>
> >> +#include <asm/suspend.h>
> >> +
> >> +#include "pm.h"
> >> +
> >> +static u32 sirfsoc_pwrc_base;
> >> +void __iomem *sirfsoc_memc_base;
> >> +
> >> +static void sirfsoc_set_wakeup_source(void)
> >> +{
> >> + ? ? ? ?u32 pwr_trigger_en_reg;
> >> + ? ? ? ?pwr_trigger_en_reg = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
> >> + ? ? ? ? ? ? SIRFSOC_PWRC_TRIGGER_EN);
> >> +#define X_ON_KEY_B (1 << 0)
> >> + ? ? ? ?sirfsoc_rtc_iobrg_writel(pwr_trigger_en_reg | X_ON_KEY_B,
> >> + ? ? ? ? ? ? ? ? ? ? ? ?sirfsoc_pwrc_base + SIRFSOC_PWRC_TRIGGER_EN);
> >> +}
> >> +
> >> +static void sirfsoc_set_sleep_mode(u32 mode)
> >> +{
> >> + ? ? ? ?u32 sleep_mode = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
> >> + ? ? ? ? ? ? SIRFSOC_PWRC_PDN_CTRL);
> >> + ? ? ? ?sleep_mode &= ~(SIRFSOC_SLEEP_MODE_MASK << 1);
> >> + ? ? ? ?sleep_mode |= ((mode << 1));
> >
> > Redundant parentheses.
> >
> >> + ? ? ? ?sirfsoc_rtc_iobrg_writel(sleep_mode, sirfsoc_pwrc_base +
> >> + ? ? ? ? ? ? SIRFSOC_PWRC_PDN_CTRL);
> >> +}
> >> +
> >> +int sirfsoc_pre_suspend_power_off(void)
> >> +{
> >> + ? ? u32 wakeup_entry = virt_to_phys(cpu_resume);
> >> +
> >> + ? ? sirfsoc_rtc_iobrg_writel(wakeup_entry,
> >> + ? ? ? ? ? ? SIRFSOC_PWRC_SCRATCH_PAD1);
> >> +
> >> + ? ? sirfsoc_set_wakeup_source();
> >> +
> >> + ? ? sirfsoc_set_sleep_mode(SIRFSOC_DEEP_SLEEP_MODE);
> >> +
> >> + ? ? return 0;
> >> +}
> >> +
> > If I read it correctly, this function sets up resume entry, but I
> > failed to see it being called anywhere? ?Am I missing anything?
> 
> lose this while sending patch, it is before cpu_suspend() called.
> Thanks for review.
> 
> >
> >> +static void sirfsoc_save_register(u32 *ptr)
> >> +{
> >> + ? ? /* todo: save necessary system registers here */
> >> +}
> >> +
> >> +static void sirfsoc_restore_regs(u32 *ptr)
> >> +{
> >> + ? ? /* todo: restore saved system registers here */
> >> +}
> >> +
> >> +static int sirfsoc_pm_enter(suspend_state_t state)
> >> +{
> >> + ? ? u32 *saved_regs;
> >> +
> >> + ? ? switch (state) {
> >> + ? ? case PM_SUSPEND_MEM:
> >> + ? ? ? ? ? ? saved_regs = kmalloc(1024, GFP_ATOMIC);
> >> + ? ? ? ? ? ? if (!saved_regs)
> >> + ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
> >> +
> >> + ? ? ? ? ? ? sirfsoc_save_register(saved_regs);
> >> +
> >> + ? ? ? ? ? ? /* go zzz */
> >> + ? ? ? ? ? ? cpu_suspend(0, sirfsoc_finish_suspend);
> >> +
> >> + ? ? ? ? ? ? sirfsoc_restore_regs(saved_regs);
> >> + ? ? ? ? ? ? kfree(saved_regs);
> >> + ? ? ? ? ? ? break;
> >> + ? ? default:
> >> + ? ? ? ? ? ? return -EINVAL;
> >> + ? ? }
> >> + ? ? return 0;
> >> +}
> >> +
> >> +static const struct platform_suspend_ops sirfsoc_pm_ops = {
> >> + ? ? .enter = sirfsoc_pm_enter,
> >> + ? ? .valid = suspend_valid_only_mem,
> >> +};
> >> +
> >> +static int __init sirfsoc_pm_init(void)
> >> +{
> >> + ? ? suspend_set_ops(&sirfsoc_pm_ops);
> >> + ? ? return 0;
> >> +}
> >> +late_initcall(sirfsoc_pm_init);
> >> +
> >
> > 8<---
> >> +static struct of_device_id pwrc_ids[] = {
> >> + ? ? { .compatible = "sirf,prima2-pwrc" },
> >> +};
> >> +
> >> +static int __init sirfsoc_of_pwrc_init(void)
> >> +{
> >> + ? ? struct device_node *np;
> >> + ? ? const __be32 ? ?*addrp;
> >> +
> >> + ? ? np = of_find_matching_node(NULL, pwrc_ids);
> >> + ? ? if (!np)
> >> + ? ? ? ? ? ? panic("unable to find compatible pwrc node in dtb\n");
> >> +
> >> + ? ? addrp = of_get_property(np, "reg", NULL);
> >> + ? ? if (!addrp)
> >> + ? ? ? ? ? ? panic("unable to find base address of pwrc node in dtb\n");
> >> +
> >> + ? ? sirfsoc_pwrc_base = be32_to_cpup(addrp);
> >> +
> >> + ? ? of_node_put(np);
> > --->8
> >
> > To me, it seems that the above code snippet can be as simply as the
> > following.
> >
> > ? ? ? ?np = of_find_compatible_node(NULL, NULL, "sirf,prima2-pwrc");
> > ? ? ? ?sirfsoc_pwrc_base = of_iomap(np, 0);
> > ? ? ? ?if (!sirfsoc_pwrc_base)
> > ? ? ? ? ? ? ? ?return -ENOMEM;
> 
> you might want to read earlier thread in v1. PWRC is not in memory space.
> 
Ah, just read.  Then you at least should be able to use
of_property_read_u32().

> >
> >> +
> >> + ? ? return 0;
> >> +}
> >> +postcore_initcall(sirfsoc_of_pwrc_init);
> >> +
> >> +static struct of_device_id memc_ids[] = {
> >> + ? ? { .compatible = "sirf,prima2-memc" },
> >> +};
> >> +
> >> +static int __devinit sirfsoc_memc_probe(struct platform_device *op)
> >> +{
> >> + ? ? struct device_node *np = op->dev.of_node;
> >> +
> >> + ? ? sirfsoc_memc_base = of_iomap(np, 0);
> >> + ? ? if (!sirfsoc_memc_base)
> >> + ? ? ? ? ? ? panic("unable to map memc registers\n");
> >> +
> >> + ? ? return 0;
> >> +}
> >> +
> >> +static struct platform_driver sirfsoc_memc_driver = {
> >> + ? ? .probe ? ? ? ? ?= sirfsoc_memc_probe,
> >> + ? ? .driver = {
> >> + ? ? ? ? ? ? .name = "sirfsoc-memc",
> >> + ? ? ? ? ? ? .owner = THIS_MODULE,
> >> + ? ? ? ? ? ? .of_match_table = memc_ids,
> >> + ? ? },
> >> +};
> >> +
> >> +static int __init sirfsoc_memc_init(void)
> >> +{
> >> + ? ? return platform_driver_register(&sirfsoc_memc_driver);
> >> +}
> >> +postcore_initcall(sirfsoc_memc_init);
> >
> > Doing the same thing - mapping address, why sirfsoc_pwrc_base uses
> > a function, while sirfsoc_memc_base needs a platform_driver? ?You
> > will have more stuff about memc to add there?
> 
> memc is in memory space, actually simple_bus, then a platform device
> has existed for it.
> pwrc is now not compitable with simple_bus. it looks like not worth a
> platform for the moment too.
> 
It seems a little complicated to register a platform_driver just for
getting an address.  I'm not sure how hard Arnd is on this position.
I'm going to send a patch to test it :)

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-25  8:21       ` Shawn Guo
@ 2011-08-25  8:21         ` Barry Song
  2011-08-25  8:33           ` Barry Song
  2011-08-25 15:56         ` Arnd Bergmann
  1 sibling, 1 reply; 25+ messages in thread
From: Barry Song @ 2011-08-25  8:21 UTC (permalink / raw)
  To: linux-arm-kernel

2011/8/25 Shawn Guo <shawn.guo@freescale.com>:
> On Thu, Aug 25, 2011 at 03:30:27PM +0800, Barry Song wrote:
>> Hi Shawn,
>> 2011/8/25 Shawn Guo <shawn.guo@freescale.com>:
>> > On Mon, Aug 22, 2011 at 11:15:50PM -0700, Barry Song wrote:
>> >> From: Rongjun Ying <rongjun.ying@csr.com>
>> >>
>> >> Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
>> >> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> >> ---
>> >> ?-v2:
>> >> ?delete redundant ARM mode registers save/restore
>> >> ?use generic cpu suspend
>> >> ?move l2 cache suspend/resume codes out
>> >> ?make memc become a device driver
>> >>
>> >> ?arch/arm/mach-prima2/pm.c ? ?| ?158 ++++++++++++++++++++++++++++++++++++++++++
>> >> ?arch/arm/mach-prima2/pm.h ? ?| ? 31 ++++++++
>> >> ?arch/arm/mach-prima2/sleep.S | ? 61 ++++++++++++++++
>> >> ?3 files changed, 250 insertions(+), 0 deletions(-)
>> >> ?create mode 100644 arch/arm/mach-prima2/pm.c
>> >> ?create mode 100644 arch/arm/mach-prima2/pm.h
>> >> ?create mode 100644 arch/arm/mach-prima2/sleep.S
>> >>
>> >> diff --git a/arch/arm/mach-prima2/pm.c b/arch/arm/mach-prima2/pm.c
>> >> new file mode 100644
>> >> index 0000000..75167df
>> >> --- /dev/null
>> >> +++ b/arch/arm/mach-prima2/pm.c
>> >> @@ -0,0 +1,158 @@
>> >> +/*
>> >> + * power management entry for CSR SiRFprimaII
>> >> + *
>> >> + * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
>> >> + *
>> >> + * Licensed under GPLv2 or later.
>> >> + */
>> >> +
>> >> +#include <linux/kernel.h>
>> >> +#include <linux/suspend.h>
>> >> +#include <linux/slab.h>
>> >> +#include <linux/of.h>
>> >> +#include <linux/of_address.h>
>> >> +#include <linux/of_device.h>
>> >> +#include <linux/of_platform.h>
>> >> +#include <linux/io.h>
>> >> +#include <linux/rtc/sirfsoc_rtciobrg.h>
>> >> +#include <asm/suspend.h>
>> >> +
>> >> +#include "pm.h"
>> >> +
>> >> +static u32 sirfsoc_pwrc_base;
>> >> +void __iomem *sirfsoc_memc_base;
>> >> +
>> >> +static void sirfsoc_set_wakeup_source(void)
>> >> +{
>> >> + ? ? ? ?u32 pwr_trigger_en_reg;
>> >> + ? ? ? ?pwr_trigger_en_reg = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
>> >> + ? ? ? ? ? ? SIRFSOC_PWRC_TRIGGER_EN);
>> >> +#define X_ON_KEY_B (1 << 0)
>> >> + ? ? ? ?sirfsoc_rtc_iobrg_writel(pwr_trigger_en_reg | X_ON_KEY_B,
>> >> + ? ? ? ? ? ? ? ? ? ? ? ?sirfsoc_pwrc_base + SIRFSOC_PWRC_TRIGGER_EN);
>> >> +}
>> >> +
>> >> +static void sirfsoc_set_sleep_mode(u32 mode)
>> >> +{
>> >> + ? ? ? ?u32 sleep_mode = sirfsoc_rtc_iobrg_readl(sirfsoc_pwrc_base +
>> >> + ? ? ? ? ? ? SIRFSOC_PWRC_PDN_CTRL);
>> >> + ? ? ? ?sleep_mode &= ~(SIRFSOC_SLEEP_MODE_MASK << 1);
>> >> + ? ? ? ?sleep_mode |= ((mode << 1));
>> >
>> > Redundant parentheses.
>> >
>> >> + ? ? ? ?sirfsoc_rtc_iobrg_writel(sleep_mode, sirfsoc_pwrc_base +
>> >> + ? ? ? ? ? ? SIRFSOC_PWRC_PDN_CTRL);
>> >> +}
>> >> +
>> >> +int sirfsoc_pre_suspend_power_off(void)
>> >> +{
>> >> + ? ? u32 wakeup_entry = virt_to_phys(cpu_resume);
>> >> +
>> >> + ? ? sirfsoc_rtc_iobrg_writel(wakeup_entry,
>> >> + ? ? ? ? ? ? SIRFSOC_PWRC_SCRATCH_PAD1);
>> >> +
>> >> + ? ? sirfsoc_set_wakeup_source();
>> >> +
>> >> + ? ? sirfsoc_set_sleep_mode(SIRFSOC_DEEP_SLEEP_MODE);
>> >> +
>> >> + ? ? return 0;
>> >> +}
>> >> +
>> > If I read it correctly, this function sets up resume entry, but I
>> > failed to see it being called anywhere? ?Am I missing anything?
>>
>> lose this while sending patch, it is before cpu_suspend() called.
>> Thanks for review.
>>
>> >
>> >> +static void sirfsoc_save_register(u32 *ptr)
>> >> +{
>> >> + ? ? /* todo: save necessary system registers here */
>> >> +}
>> >> +
>> >> +static void sirfsoc_restore_regs(u32 *ptr)
>> >> +{
>> >> + ? ? /* todo: restore saved system registers here */
>> >> +}
>> >> +
>> >> +static int sirfsoc_pm_enter(suspend_state_t state)
>> >> +{
>> >> + ? ? u32 *saved_regs;
>> >> +
>> >> + ? ? switch (state) {
>> >> + ? ? case PM_SUSPEND_MEM:
>> >> + ? ? ? ? ? ? saved_regs = kmalloc(1024, GFP_ATOMIC);
>> >> + ? ? ? ? ? ? if (!saved_regs)
>> >> + ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
>> >> +
>> >> + ? ? ? ? ? ? sirfsoc_save_register(saved_regs);
>> >> +
>> >> + ? ? ? ? ? ? /* go zzz */
>> >> + ? ? ? ? ? ? cpu_suspend(0, sirfsoc_finish_suspend);
>> >> +
>> >> + ? ? ? ? ? ? sirfsoc_restore_regs(saved_regs);
>> >> + ? ? ? ? ? ? kfree(saved_regs);
>> >> + ? ? ? ? ? ? break;
>> >> + ? ? default:
>> >> + ? ? ? ? ? ? return -EINVAL;
>> >> + ? ? }
>> >> + ? ? return 0;
>> >> +}
>> >> +
>> >> +static const struct platform_suspend_ops sirfsoc_pm_ops = {
>> >> + ? ? .enter = sirfsoc_pm_enter,
>> >> + ? ? .valid = suspend_valid_only_mem,
>> >> +};
>> >> +
>> >> +static int __init sirfsoc_pm_init(void)
>> >> +{
>> >> + ? ? suspend_set_ops(&sirfsoc_pm_ops);
>> >> + ? ? return 0;
>> >> +}
>> >> +late_initcall(sirfsoc_pm_init);
>> >> +
>> >
>> > 8<---
>> >> +static struct of_device_id pwrc_ids[] = {
>> >> + ? ? { .compatible = "sirf,prima2-pwrc" },
>> >> +};
>> >> +
>> >> +static int __init sirfsoc_of_pwrc_init(void)
>> >> +{
>> >> + ? ? struct device_node *np;
>> >> + ? ? const __be32 ? ?*addrp;
>> >> +
>> >> + ? ? np = of_find_matching_node(NULL, pwrc_ids);
>> >> + ? ? if (!np)
>> >> + ? ? ? ? ? ? panic("unable to find compatible pwrc node in dtb\n");
>> >> +
>> >> + ? ? addrp = of_get_property(np, "reg", NULL);
>> >> + ? ? if (!addrp)
>> >> + ? ? ? ? ? ? panic("unable to find base address of pwrc node in dtb\n");
>> >> +
>> >> + ? ? sirfsoc_pwrc_base = be32_to_cpup(addrp);
>> >> +
>> >> + ? ? of_node_put(np);
>> > --->8
>> >
>> > To me, it seems that the above code snippet can be as simply as the
>> > following.
>> >
>> > ? ? ? ?np = of_find_compatible_node(NULL, NULL, "sirf,prima2-pwrc");
>> > ? ? ? ?sirfsoc_pwrc_base = of_iomap(np, 0);
>> > ? ? ? ?if (!sirfsoc_pwrc_base)
>> > ? ? ? ? ? ? ? ?return -ENOMEM;
>>
>> you might want to read earlier thread in v1. PWRC is not in memory space.
>>
> Ah, just read. ?Then you at least should be able to use
> of_property_read_u32().
>
>> >
>> >> +
>> >> + ? ? return 0;
>> >> +}
>> >> +postcore_initcall(sirfsoc_of_pwrc_init);
>> >> +
>> >> +static struct of_device_id memc_ids[] = {
>> >> + ? ? { .compatible = "sirf,prima2-memc" },
>> >> +};
>> >> +
>> >> +static int __devinit sirfsoc_memc_probe(struct platform_device *op)
>> >> +{
>> >> + ? ? struct device_node *np = op->dev.of_node;
>> >> +
>> >> + ? ? sirfsoc_memc_base = of_iomap(np, 0);
>> >> + ? ? if (!sirfsoc_memc_base)
>> >> + ? ? ? ? ? ? panic("unable to map memc registers\n");
>> >> +
>> >> + ? ? return 0;
>> >> +}
>> >> +
>> >> +static struct platform_driver sirfsoc_memc_driver = {
>> >> + ? ? .probe ? ? ? ? ?= sirfsoc_memc_probe,
>> >> + ? ? .driver = {
>> >> + ? ? ? ? ? ? .name = "sirfsoc-memc",
>> >> + ? ? ? ? ? ? .owner = THIS_MODULE,
>> >> + ? ? ? ? ? ? .of_match_table = memc_ids,
>> >> + ? ? },
>> >> +};
>> >> +
>> >> +static int __init sirfsoc_memc_init(void)
>> >> +{
>> >> + ? ? return platform_driver_register(&sirfsoc_memc_driver);
>> >> +}
>> >> +postcore_initcall(sirfsoc_memc_init);
>> >
>> > Doing the same thing - mapping address, why sirfsoc_pwrc_base uses
>> > a function, while sirfsoc_memc_base needs a platform_driver? ?You
>> > will have more stuff about memc to add there?
>>
>> memc is in memory space, actually simple_bus, then a platform device
>> has existed for it.
>> pwrc is now not compitable with simple_bus. it looks like not worth a
>> platform for the moment too.
>>
> It seems a little complicated to register a platform_driver just for
> getting an address. ?I'm not sure how hard Arnd is on this position.
> I'm going to send a patch to test it :)

i think Arnd preferred it to be driver:

"
> +static void __init sirfsoc_of_memc_map(void)
> +{
> +     struct device_node *np;
> +
> +     np = of_find_matching_node(NULL, memc_ids);
> +     if (!np)
> +             panic("unable to find compatible memc node in dtb\n");
> +
> +     sirfsoc_memc_base = of_iomap(np, 0);
> +     if (!np)
> +             panic("unable to map compatible memc node in dtb\n");
> +
> +     of_node_put(np);
> +}

Same as for the other one, I think this should be another device
driver.
"
>
> --
> Regards,
> Shawn

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-25  8:21         ` Barry Song
@ 2011-08-25  8:33           ` Barry Song
  0 siblings, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-25  8:33 UTC (permalink / raw)
  To: linux-arm-kernel

>>> >> +static int __devinit sirfsoc_memc_probe(struct platform_device *op)
>>> >> +{
>>> >> + ? ? struct device_node *np = op->dev.of_node;
>>> >> +
>>> >> + ? ? sirfsoc_memc_base = of_iomap(np, 0);
>>> >> + ? ? if (!sirfsoc_memc_base)
>>> >> + ? ? ? ? ? ? panic("unable to map memc registers\n");
>>> >> +
>>> >> + ? ? return 0;
>>> >> +}
>>> >> +
>>> >> +static struct platform_driver sirfsoc_memc_driver = {
>>> >> + ? ? .probe ? ? ? ? ?= sirfsoc_memc_probe,
>>> >> + ? ? .driver = {
>>> >> + ? ? ? ? ? ? .name = "sirfsoc-memc",
>>> >> + ? ? ? ? ? ? .owner = THIS_MODULE,
>>> >> + ? ? ? ? ? ? .of_match_table = memc_ids,
>>> >> + ? ? },
>>> >> +};
>>> >> +
>>> >> +static int __init sirfsoc_memc_init(void)
>>> >> +{
>>> >> + ? ? return platform_driver_register(&sirfsoc_memc_driver);
>>> >> +}
>>> >> +postcore_initcall(sirfsoc_memc_init);
>>> >
>>> > Doing the same thing - mapping address, why sirfsoc_pwrc_base uses
>>> > a function, while sirfsoc_memc_base needs a platform_driver? ?You
>>> > will have more stuff about memc to add there?
>>>
>>> memc is in memory space, actually simple_bus, then a platform device
>>> has existed for it.
>>> pwrc is now not compitable with simple_bus. it looks like not worth a
>>> platform for the moment too.
>>>
>> It seems a little complicated to register a platform_driver just for
>> getting an address. ?I'm not sure how hard Arnd is on this position.
>> I'm going to send a patch to test it :)
>
> i think Arnd preferred it to be driver:
>
> "
>> +static void __init sirfsoc_of_memc_map(void)
>> +{
>> + ? ? struct device_node *np;
>> +
>> + ? ? np = of_find_matching_node(NULL, memc_ids);
>> + ? ? if (!np)
>> + ? ? ? ? ? ? panic("unable to find compatible memc node in dtb\n");
>> +
>> + ? ? sirfsoc_memc_base = of_iomap(np, 0);
>> + ? ? if (!np)
>> + ? ? ? ? ? ? panic("unable to map compatible memc node in dtb\n");
>> +
>> + ? ? of_node_put(np);
>> +}
>
> Same as for the other one, I think this should be another device
> driver.
> "

will we have a common wrapper as below if we only need some virtual
address and have no complicated driver?

void __iomem* of_iomap_from_id(struct of_device_id id[], int index)
{
        struct device_node *np;
        void iomem *addr;

        np = of_find_matching_node(NULL, ids);
        if (!np)
                return NULL;

        addr = of_iomap(ids, 0);

        of_node_put(np);

        return addr;
}

-barry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-25  8:21       ` Shawn Guo
  2011-08-25  8:21         ` Barry Song
@ 2011-08-25 15:56         ` Arnd Bergmann
  2011-08-25 22:58           ` Barry Song
  1 sibling, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2011-08-25 15:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday 25 August 2011, Shawn Guo wrote:
> > > To me, it seems that the above code snippet can be as simply as the
> > > following.
> > >
> > >        np = of_find_compatible_node(NULL, NULL, "sirf,prima2-pwrc");
> > >        sirfsoc_pwrc_base = of_iomap(np, 0);
> > >        if (!sirfsoc_pwrc_base)
> > >                return -ENOMEM;
> > 
> > you might want to read earlier thread in v1. PWRC is not in memory space.
> > 
> Ah, just read.  Then you at least should be able to use
> of_property_read_u32().

I think of_get_address would be even more appropriate, but I could
be misreading that function.

> > >> +
> > >> +static int __init sirfsoc_memc_init(void)
> > >> +{
> > >> +     return platform_driver_register(&sirfsoc_memc_driver);
> > >> +}
> > >> +postcore_initcall(sirfsoc_memc_init);
> > >
> > > Doing the same thing - mapping address, why sirfsoc_pwrc_base uses
> > > a function, while sirfsoc_memc_base needs a platform_driver?  You
> > > will have more stuff about memc to add there?
> > 
> > memc is in memory space, actually simple_bus, then a platform device
> > has existed for it.
> > pwrc is now not compitable with simple_bus. it looks like not worth a
> > platform for the moment too.
> > 
> It seems a little complicated to register a platform_driver just for
> getting an address.  I'm not sure how hard Arnd is on this position.
> I'm going to send a patch to test it :)

I think it's a grey area. In general, I always recommend a device
driver unless the mapping is absolutely needed at boot time before
we bring up the driver subsystem.
This enforces an object-oriented mental model about the building
blocks: Everything you want to talk to has to export its own functions
and we can stack modules on top of other modules. Clearly there are
a few cases where this is not possible or only adds complexity without
any benefit, but I would like to see the model of having a device
driver for each device be the rule, with few exceptions.

One argument that I can accept for this specific case is that the
power management code has to be written in assembly and doesn't
really understand the device object model anyway, so we just end
up exporting the base address, which is not something that proper
drivers are doing. However, as soon as more functionality gets added
that uses the memc register space, my preference would increasingly
lean towards doing a device driver here.

	Arnd

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-25 15:56         ` Arnd Bergmann
@ 2011-08-25 22:58           ` Barry Song
  0 siblings, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-25 22:58 UTC (permalink / raw)
  To: linux-arm-kernel

2011/8/25 Arnd Bergmann <arnd@arndb.de>:
> On Thursday 25 August 2011, Shawn Guo wrote:
>> > > To me, it seems that the above code snippet can be as simply as the
>> > > following.
>> > >
>> > > ? ? ? ?np = of_find_compatible_node(NULL, NULL, "sirf,prima2-pwrc");
>> > > ? ? ? ?sirfsoc_pwrc_base = of_iomap(np, 0);
>> > > ? ? ? ?if (!sirfsoc_pwrc_base)
>> > > ? ? ? ? ? ? ? ?return -ENOMEM;
>> >
>> > you might want to read earlier thread in v1. PWRC is not in memory space.
>> >
>> Ah, just read. ?Then you at least should be able to use
>> of_property_read_u32().
>
> I think of_get_address would be even more appropriate, but I could
> be misreading that function.

might use of_get_address and add some comments to clarify.

>
>> > >> +
>> > >> +static int __init sirfsoc_memc_init(void)
>> > >> +{
>> > >> + ? ? return platform_driver_register(&sirfsoc_memc_driver);
>> > >> +}
>> > >> +postcore_initcall(sirfsoc_memc_init);
>> > >
>> > > Doing the same thing - mapping address, why sirfsoc_pwrc_base uses
>> > > a function, while sirfsoc_memc_base needs a platform_driver? ?You
>> > > will have more stuff about memc to add there?
>> >
>> > memc is in memory space, actually simple_bus, then a platform device
>> > has existed for it.
>> > pwrc is now not compitable with simple_bus. it looks like not worth a
>> > platform for the moment too.
>> >
>> It seems a little complicated to register a platform_driver just for
>> getting an address. ?I'm not sure how hard Arnd is on this position.
>> I'm going to send a patch to test it :)
>
> I think it's a grey area. In general, I always recommend a device
> driver unless the mapping is absolutely needed at boot time before
> we bring up the driver subsystem.
> This enforces an object-oriented mental model about the building
> blocks: Everything you want to talk to has to export its own functions
> and we can stack modules on top of other modules. Clearly there are
> a few cases where this is not possible or only adds complexity without
> any benefit, but I would like to see the model of having a device
> driver for each device be the rule, with few exceptions.
>
> One argument that I can accept for this specific case is that the
> power management code has to be written in assembly and doesn't
> really understand the device object model anyway, so we just end
> up exporting the base address, which is not something that proper
> drivers are doing. However, as soon as more functionality gets added
> that uses the memc register space, my preference would increasingly
> lean towards doing a device driver here.
>
> ? ? ? ?Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Thanks
barry

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/4] ARM: CSR: add PM sleep entry for SiRFprimaII
  2011-08-23  9:41   ` Russell King - ARM Linux
@ 2011-08-27  9:53     ` Barry Song
  0 siblings, 0 replies; 25+ messages in thread
From: Barry Song @ 2011-08-27  9:53 UTC (permalink / raw)
  To: linux-arm-kernel

2011/8/23 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Mon, Aug 22, 2011 at 11:15:50PM -0700, Barry Song wrote:
>> From: Rongjun Ying <rongjun.ying@csr.com>
>>
>> Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>> ?-v2:
>> ?delete redundant ARM mode registers save/restore
>> ?use generic cpu suspend
>> ?move l2 cache suspend/resume codes out
>> ?make memc become a device driver

Arnd, are you ok with this or you have more comments? if you have been
ok, i'd like to send v3 with fixing comments from Jamie, Shawn and
you.

-barry

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2011-08-27  9:53 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-23  6:15 [PATCH v2 0/4] ARM: CSR: add rtciobrg and PM support Barry Song
2011-08-23  6:15 ` [PATCH v2 1/4] ARM: CSR: add rtc i/o bridge interface for SiRFprimaII Barry Song
2011-08-23  8:48   ` Jamie Iles
2011-08-23  9:33     ` Barry Song
2011-08-23  9:53       ` Jamie Iles
2011-08-23 15:59         ` Arnd Bergmann
2011-08-24  1:45           ` Barry Song
2011-08-23 16:01   ` Arnd Bergmann
2011-08-23 16:15     ` Arnd Bergmann
2011-08-24  1:43       ` Barry Song
2011-08-24  1:42     ` Barry Song
2011-08-23  6:15 ` [PATCH v2 2/4] ARM: CSR: add PM sleep entry " Barry Song
2011-08-23  8:50   ` Jamie Iles
2011-08-23  9:08     ` Barry Song
2011-08-23  9:41   ` Russell King - ARM Linux
2011-08-27  9:53     ` Barry Song
2011-08-25  7:27   ` Shawn Guo
2011-08-25  7:30     ` Barry Song
2011-08-25  8:21       ` Shawn Guo
2011-08-25  8:21         ` Barry Song
2011-08-25  8:33           ` Barry Song
2011-08-25 15:56         ` Arnd Bergmann
2011-08-25 22:58           ` Barry Song
2011-08-23  6:15 ` [PATCH v2 3/4] ARM: L2X0: move l2x0_init out of .init section Barry Song
2011-08-23  6:15 ` [PATCH v2 4/4] ARM: CSR: add PM suspend/resume entries for SiRFprimaII L2 cache Barry Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.