Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 04/21] ARM: tegra: clock: Don't use PLL lock bits
From: Colin Cross @ 2011-02-19 22:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298154371-5641-1-git-send-email-ccross@android.com>

The PLL lock bits are not reliable, use per-PLL timeouts instead.

Signed-off-by: Colin Cross <ccross@android.com>
---
 arch/arm/mach-tegra/clock.h         |    2 +-
 arch/arm/mach-tegra/tegra2_clocks.c |   31 +++++++++----------------------
 2 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-tegra/clock.h b/arch/arm/mach-tegra/clock.h
index 42f00c0..b76d33d 100644
--- a/arch/arm/mach-tegra/clock.h
+++ b/arch/arm/mach-tegra/clock.h
@@ -53,7 +53,6 @@ struct dvfs_process_id_table {
 	struct dvfs_table *table;
 };
 
-
 struct dvfs {
 	struct regulator *reg;
 	struct dvfs_table *table;
@@ -128,6 +127,7 @@ struct clk {
 	unsigned long			vco_min;
 	unsigned long			vco_max;
 	const struct clk_pll_table	*pll_table;
+	int				pll_lock_delay;
 
 	/* DIV */
 	u32				div;
diff --git a/arch/arm/mach-tegra/tegra2_clocks.c b/arch/arm/mach-tegra/tegra2_clocks.c
index db27e59..36da2e8 100644
--- a/arch/arm/mach-tegra/tegra2_clocks.c
+++ b/arch/arm/mach-tegra/tegra2_clocks.c
@@ -79,7 +79,6 @@
 #define PLL_BASE_ENABLE			(1<<30)
 #define PLL_BASE_REF_ENABLE		(1<<29)
 #define PLL_BASE_OVERRIDE		(1<<28)
-#define PLL_BASE_LOCK			(1<<27)
 #define PLL_BASE_DIVP_MASK		(0x7<<20)
 #define PLL_BASE_DIVP_SHIFT		20
 #define PLL_BASE_DIVN_MASK		(0x3FF<<8)
@@ -94,7 +93,6 @@
 #define PLL_OUT_RESET_DISABLE		(1<<0)
 
 #define PLL_MISC(c)			(((c)->flags & PLL_ALT_MISC_REG) ? 0x4 : 0xc)
-#define PLL_MISC_LOCK_ENABLE(c)		(((c)->flags & PLLU) ? (1<<22) : (1<<18))
 
 #define PLL_MISC_DCCON_SHIFT		20
 #define PLL_MISC_CPCON_SHIFT		8
@@ -546,17 +544,7 @@ static struct clk_ops tegra_blink_clk_ops = {
 /* PLL Functions */
 static int tegra2_pll_clk_wait_for_lock(struct clk *c)
 {
-	ktime_t before;
-
-	before = ktime_get();
-
-	while (!(clk_readl(c->reg + PLL_BASE) & PLL_BASE_LOCK)) {
-		if (ktime_us_delta(ktime_get(), before) > 5000) {
-			pr_err("Timed out waiting for lock bit on pll %s",
-				c->name);
-			return -1;
-		}
-	}
+	udelay(c->pll_lock_delay);
 
 	return 0;
 }
@@ -594,10 +582,6 @@ static int tegra2_pll_clk_enable(struct clk *c)
 	val |= PLL_BASE_ENABLE;
 	clk_writel(val, c->reg + PLL_BASE);
 
-	val = clk_readl(c->reg + PLL_MISC(c));
-	val |= PLL_MISC_LOCK_ENABLE(c);
-	clk_writel(val, c->reg + PLL_MISC(c));
-
 	tegra2_pll_clk_wait_for_lock(c);
 
 	return 0;
@@ -1177,6 +1161,7 @@ static struct clk tegra_pll_s = {
 	.vco_max   = 26000000,
 	.pll_table = tegra_pll_s_table,
 	.max_rate  = 26000000,
+	.pll_lock_delay = 300,
 };
 
 static struct clk_mux_sel tegra_clk_m_sel[] = {
@@ -1213,6 +1198,7 @@ static struct clk tegra_pll_c = {
 	.vco_max   = 1400000000,
 	.pll_table = tegra_pll_c_table,
 	.max_rate  = 600000000,
+	.pll_lock_delay = 300,
 };
 
 static struct clk tegra_pll_c_out1 = {
@@ -1251,6 +1237,7 @@ static struct clk tegra_pll_m = {
 	.vco_max   = 1200000000,
 	.pll_table = tegra_pll_m_table,
 	.max_rate  = 800000000,
+	.pll_lock_delay = 300,
 };
 
 static struct clk tegra_pll_m_out1 = {
@@ -1289,6 +1276,7 @@ static struct clk tegra_pll_p = {
 	.vco_max   = 1400000000,
 	.pll_table = tegra_pll_p_table,
 	.max_rate  = 432000000,
+	.pll_lock_delay = 300,
 };
 
 static struct clk tegra_pll_p_out1 = {
@@ -1354,6 +1342,7 @@ static struct clk tegra_pll_a = {
 	.vco_max   = 1400000000,
 	.pll_table = tegra_pll_a_table,
 	.max_rate  = 56448000,
+	.pll_lock_delay = 300,
 };
 
 static struct clk tegra_pll_a_out0 = {
@@ -1399,6 +1388,7 @@ static struct clk tegra_pll_d = {
 	.vco_max   = 1000000000,
 	.pll_table = tegra_pll_d_table,
 	.max_rate  = 1000000000,
+	.pll_lock_delay = 1000,
 };
 
 static struct clk tegra_pll_d_out0 = {
@@ -1431,6 +1421,7 @@ static struct clk tegra_pll_u = {
 	.vco_max   = 960000000,
 	.pll_table = tegra_pll_u_table,
 	.max_rate  = 480000000,
+	.pll_lock_delay = 1000,
 };
 
 static struct clk_pll_table tegra_pll_x_table[] = {
@@ -1493,6 +1484,7 @@ static struct clk tegra_pll_x = {
 	.vco_max   = 1200000000,
 	.pll_table = tegra_pll_x_table,
 	.max_rate  = 1000000000,
+	.pll_lock_delay = 300,
 };
 
 static struct clk_pll_table tegra_pll_e_table[] = {
@@ -1966,7 +1958,6 @@ static u32 clk_rst_suspend[RST_DEVICES_NUM + CLK_OUT_ENB_NUM +
 void tegra_clk_suspend(void)
 {
 	unsigned long off, i;
-	u32 pllx_misc;
 	u32 *ctx = clk_rst_suspend;
 
 	*ctx++ = clk_readl(OSC_CTRL) & OSC_CTRL_MASK;
@@ -2007,10 +1998,6 @@ void tegra_clk_suspend(void)
 
 	*ctx++ = clk_readl(MISC_CLK_ENB);
 	*ctx++ = clk_readl(CLK_MASK_ARM);
-
-	pllx_misc = clk_readl(tegra_pll_x.reg + PLL_MISC(&tegra_pll_x));
-	pllx_misc &= ~PLL_MISC_LOCK_ENABLE(&tegra_pll_x);
-	clk_writel(pllx_misc, tegra_pll_x.reg + PLL_MISC(&tegra_pll_x));
 }
 
 void tegra_clk_resume(void)
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH v2 03/21] ARM: tegra: clock: Drop debugging
From: Colin Cross @ 2011-02-19 22:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298154371-5641-1-git-send-email-ccross@android.com>

Signed-off-by: Colin Cross <ccross@android.com>
---
 arch/arm/mach-tegra/clock.c |   19 ++-----------------
 1 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-tegra/clock.c b/arch/arm/mach-tegra/clock.c
index 77948e0..f55bb83 100644
--- a/arch/arm/mach-tegra/clock.c
+++ b/arch/arm/mach-tegra/clock.c
@@ -138,7 +138,6 @@ static void clk_recalculate_rate(struct clk *c)
 
 int clk_reparent(struct clk *c, struct clk *parent)
 {
-	pr_debug("%s: %s\n", __func__, c->name);
 	c->parent = parent;
 	list_del(&c->sibling);
 	list_add_tail(&c->sibling, &parent->children);
@@ -148,9 +147,8 @@ int clk_reparent(struct clk *c, struct clk *parent)
 static void propagate_rate(struct clk *c)
 {
 	struct clk *clkp;
-	pr_debug("%s: %s\n", __func__, c->name);
+
 	list_for_each_entry(clkp, &c->children, sibling) {
-		pr_debug("   %s\n", clkp->name);
 		clk_recalculate_rate(clkp);
 		propagate_rate(clkp);
 	}
@@ -160,8 +158,6 @@ void clk_init(struct clk *c)
 {
 	unsigned long flags;
 
-	pr_debug("%s: %s\n", __func__, c->name);
-
 	spin_lock_irqsave(&clock_lock, flags);
 
 	INIT_LIST_HEAD(&c->children);
@@ -183,7 +179,7 @@ void clk_init(struct clk *c)
 int clk_enable_locked(struct clk *c)
 {
 	int ret;
-	pr_debug("%s: %s\n", __func__, c->name);
+
 	if (c->refcnt == 0) {
 		if (c->parent) {
 			ret = clk_enable_locked(c->parent);
@@ -247,7 +243,6 @@ EXPORT_SYMBOL(clk_enable);
 
 void clk_disable_locked(struct clk *c)
 {
-	pr_debug("%s: %s\n", __func__, c->name);
 	if (c->refcnt == 0) {
 		WARN(1, "Attempting to disable clock %s with refcnt 0", c->name);
 		return;
@@ -298,8 +293,6 @@ int clk_set_parent_locked(struct clk *c, struct clk *parent)
 {
 	int ret;
 
-	pr_debug("%s: %s\n", __func__, c->name);
-
 	if (!c->ops || !c->ops->set_parent)
 		return -ENOSYS;
 
@@ -359,8 +352,6 @@ int clk_set_rate_cansleep(struct clk *c, unsigned long rate)
 	int ret = 0;
 	unsigned long flags;
 
-	pr_debug("%s: %s\n", __func__, c->name);
-
 	mutex_lock(&dvfs_lock);
 
 	if (rate > c->rate)
@@ -388,8 +379,6 @@ int clk_set_rate(struct clk *c, unsigned long rate)
 	int ret = 0;
 	unsigned long flags;
 
-	pr_debug("%s: %s\n", __func__, c->name);
-
 	if (clk_is_dvfs(c))
 		BUG();
 
@@ -408,8 +397,6 @@ unsigned long clk_get_rate(struct clk *c)
 
 	spin_lock_irqsave(&clock_lock, flags);
 
-	pr_debug("%s: %s\n", __func__, c->name);
-
 	ret = c->rate;
 
 	spin_unlock_irqrestore(&clock_lock, flags);
@@ -419,8 +406,6 @@ EXPORT_SYMBOL(clk_get_rate);
 
 long clk_round_rate(struct clk *c, unsigned long rate)
 {
-	pr_debug("%s: %s\n", __func__, c->name);
-
 	if (!c->ops || !c->ops->round_rate)
 		return -ENOSYS;
 
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH v2 02/21] ARM: tegra: clock: Don't BUG on changing an enabled PLL
From: Colin Cross @ 2011-02-19 22:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298154371-5641-1-git-send-email-ccross@android.com>

Signed-off-by: Colin Cross <ccross@android.com>
---
 arch/arm/mach-tegra/tegra2_clocks.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-tegra/tegra2_clocks.c b/arch/arm/mach-tegra/tegra2_clocks.c
index 240f921..db27e59 100644
--- a/arch/arm/mach-tegra/tegra2_clocks.c
+++ b/arch/arm/mach-tegra/tegra2_clocks.c
@@ -620,7 +620,6 @@ static int tegra2_pll_clk_set_rate(struct clk *c, unsigned long rate)
 	const struct clk_pll_table *sel;
 
 	pr_debug("%s: %s %lu\n", __func__, c->name, rate);
-	BUG_ON(c->refcnt != 0);
 
 	input_rate = c->parent->rate;
 	for (sel = c->pll_table; sel->input_rate != 0; sel++) {
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH v2 01/21] ARM: tegra: clock: enable clk reset for non-peripheral clocks
From: Colin Cross @ 2011-02-19 22:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298154371-5641-1-git-send-email-ccross@android.com>

From: Dima Zavin <dima@android.com>

Add a new 'reset' clk op. This can be provided for any clock,
not just peripherals.

Signed-off-by: Dima Zavin <dima@android.com>
Signed-off-by: Colin Cross <ccross@android.com>
---
 arch/arm/mach-tegra/clock.h         |    1 +
 arch/arm/mach-tegra/tegra2_clocks.c |   29 ++++++++++++++++++-----------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-tegra/clock.h b/arch/arm/mach-tegra/clock.h
index 083a4cf..42f00c0 100644
--- a/arch/arm/mach-tegra/clock.h
+++ b/arch/arm/mach-tegra/clock.h
@@ -86,6 +86,7 @@ struct clk_ops {
 	int		(*set_parent)(struct clk *, struct clk *);
 	int		(*set_rate)(struct clk *, unsigned long);
 	long		(*round_rate)(struct clk *, unsigned long);
+	void		(*reset)(struct clk *, bool);
 };
 
 enum clk_state {
diff --git a/arch/arm/mach-tegra/tegra2_clocks.c b/arch/arm/mach-tegra/tegra2_clocks.c
index 7a2926a..240f921 100644
--- a/arch/arm/mach-tegra/tegra2_clocks.c
+++ b/arch/arm/mach-tegra/tegra2_clocks.c
@@ -263,6 +263,18 @@ static struct clk_ops tegra_clk_m_ops = {
 	.disable	= tegra2_clk_m_disable,
 };
 
+void tegra2_periph_reset_assert(struct clk *c)
+{
+	BUG_ON(!c->ops->reset);
+	c->ops->reset(c, true);
+}
+
+void tegra2_periph_reset_deassert(struct clk *c)
+{
+	BUG_ON(!c->ops->reset);
+	c->ops->reset(c, false);
+}
+
 /* super clock functions */
 /* "super clocks" on tegra have two-stage muxes and a clock skipping
  * super divider.  We will ignore the clock skipping divider, since we
@@ -895,23 +907,17 @@ static void tegra2_periph_clk_disable(struct clk *c)
 		CLK_OUT_ENB_CLR + PERIPH_CLK_TO_ENB_SET_REG(c));
 }
 
-void tegra2_periph_reset_deassert(struct clk *c)
+static void tegra2_periph_clk_reset(struct clk *c, bool assert)
 {
-	pr_debug("%s on clock %s\n", __func__, c->name);
-	if (!(c->flags & PERIPH_NO_RESET))
-		clk_writel(PERIPH_CLK_TO_ENB_BIT(c),
-			RST_DEVICES_CLR + PERIPH_CLK_TO_ENB_SET_REG(c));
-}
+	unsigned long base = assert ? RST_DEVICES_SET : RST_DEVICES_CLR;
 
-void tegra2_periph_reset_assert(struct clk *c)
-{
-	pr_debug("%s on clock %s\n", __func__, c->name);
+	pr_debug("%s %s on clock %s\n", __func__,
+		 assert ? "assert" : "deassert", c->name);
 	if (!(c->flags & PERIPH_NO_RESET))
 		clk_writel(PERIPH_CLK_TO_ENB_BIT(c),
-			RST_DEVICES_SET + PERIPH_CLK_TO_ENB_SET_REG(c));
+			   base + PERIPH_CLK_TO_ENB_SET_REG(c));
 }
 
-
 static int tegra2_periph_clk_set_parent(struct clk *c, struct clk *p)
 {
 	u32 val;
@@ -1002,6 +1008,7 @@ static struct clk_ops tegra_periph_clk_ops = {
 	.set_parent		= &tegra2_periph_clk_set_parent,
 	.set_rate		= &tegra2_periph_clk_set_rate,
 	.round_rate		= &tegra2_periph_clk_round_rate,
+	.reset			= &tegra2_periph_clk_reset,
 };
 
 /* Clock doubler ops */
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH v2 00/21] Tegra clock updates for 2.6.39
From: Colin Cross @ 2011-02-19 22:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298154371-5641-1-git-send-email-ccross@android.com>

This patch series brings the Tegra clock subsystem closer to the
proposed common struct clk design, which should make converting
easier.  It also fixes a few bugs and adds new features including
memory bus scaling, disabling clocks left on by the bootloader,
and shared clocks.

V2 fixes locking issues in shared bus clocks pointed out by
Stephen Boyd.

Colin Cross (20):
      ARM: tegra: clock: Don't BUG on changing an enabled PLL
      ARM: tegra: clock: Drop debugging
      ARM: tegra: clock: Don't use PLL lock bits
      ARM: tegra: clock: Disable clocks left on by bootloader
      ARM: tegra: clock: Initialize clocks that have no enable
      ARM: tegra: clock: Drop CPU dvfs
      ARM: tegra: clock: Rearrange static clock tables
      ARM: tegra: clock: Move unshared clk struct members into union
      ARM: tegra: clock: Convert global lock to a lock per clock
      ARM: tegra: cpufreq: Take an extra reference to pllx
      ARM: tegra: clock: Add shared bus clock type
      ARM: tegra: clock: Remove unnecessary uses of #ifdef CONFIG_DEBUG_FS
      ARM: tegra: clock: Refcount periph clock enables
      ARM: tegra: clock: Round rate before setting rate
      ARM: tegra: Add external memory controller driver
      ARM: tegra: clocks: Add emc scaling
      ARM: tegra: cpufreq: Adjust memory frequency with cpu frequency
      ARM: tegra: clock: Add function to set SDMMC tap delay
      ARM: tegra: clock: Fix clock issues in suspend
      ARM: tegra: clock: Miscellaneous clock updates

Dima Zavin (1):
      ARM: tegra: clock: enable clk reset for non-peripheral clocks

 arch/arm/mach-tegra/Kconfig                        |   10 +
 arch/arm/mach-tegra/Makefile                       |    2 +-
 arch/arm/mach-tegra/clock.c                        |  559 +++++++-------
 arch/arm/mach-tegra/clock.h                        |  129 ++--
 arch/arm/mach-tegra/cpu-tegra.c                    |   25 +-
 arch/arm/mach-tegra/include/mach/clk.h             |    6 +-
 arch/arm/mach-tegra/tegra2_clocks.c                |  865 ++++++++++++++------
 arch/arm/mach-tegra/tegra2_dvfs.c                  |   86 --
 arch/arm/mach-tegra/tegra2_emc.c                   |  172 ++++
 .../arm/mach-tegra/{tegra2_dvfs.h => tegra2_emc.h} |   13 +-
 10 files changed, 1153 insertions(+), 714 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/tegra2_dvfs.c
 create mode 100644 arch/arm/mach-tegra/tegra2_emc.c
 rename arch/arm/mach-tegra/{tegra2_dvfs.h => tegra2_emc.h} (66%)

^ permalink raw reply

* [PATCH v2 12/21] ARM: tegra: clock: Add shared bus clock type
From: Colin Cross @ 2011-02-19 22:25 UTC (permalink / raw)
  To: linux-arm-kernel

Some clocks may have multiple downstream users that need to request a
higher clock rate.  Shared bus clocks provide a unique shared_bus_user
clock to each user.  The frequency of the bus is set to the highest
enabled shared_bus_user clock, with a minimum value set by the
shared bus.  Drivers can use clk_enable and clk_disable to enable
or disable their requirement, and clk_set_rate to set the minimum rate.

Signed-off-by: Colin Cross <ccross@android.com>
---
 arch/arm/mach-tegra/clock.h         |    8 +++
 arch/arm/mach-tegra/tegra2_clocks.c |  116 +++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/clock.h b/arch/arm/mach-tegra/clock.h
index a63dbf9..bb755c2 100644
--- a/arch/arm/mach-tegra/clock.h
+++ b/arch/arm/mach-tegra/clock.h
@@ -85,6 +85,7 @@ struct clk {
 	struct clk_ops		*ops;
 	unsigned long		rate;
 	unsigned long		max_rate;
+	unsigned long		min_rate;
 	u32			flags;
 	const char		*name;
 
@@ -98,6 +99,8 @@ struct clk {
 	u32				reg;
 	u32				reg_shift;
 
+	struct list_head		shared_bus_list;
+
 	union {
 		struct {
 			unsigned int			clk_num;
@@ -120,6 +123,11 @@ struct clk {
 			struct clk			*main;
 			struct clk			*backup;
 		} cpu;
+		struct {
+			struct list_head		node;
+			bool				enabled;
+			unsigned long			rate;
+		} shared_bus_user;
 	} u;
 
 	spinlock_t spinlock;
diff --git a/arch/arm/mach-tegra/tegra2_clocks.c b/arch/arm/mach-tegra/tegra2_clocks.c
index a1c86d8..dd53af3 100644
--- a/arch/arm/mach-tegra/tegra2_clocks.c
+++ b/arch/arm/mach-tegra/tegra2_clocks.c
@@ -1188,6 +1188,110 @@ static struct clk_ops tegra_cdev_clk_ops = {
 	.disable		= &tegra2_cdev_clk_disable,
 };
 
+/* shared bus ops */
+/*
+ * Some clocks may have multiple downstream users that need to request a
+ * higher clock rate.  Shared bus clocks provide a unique shared_bus_user
+ * clock to each user.  The frequency of the bus is set to the highest
+ * enabled shared_bus_user clock, with a minimum value set by the
+ * shared bus.
+ */
+static int tegra_clk_shared_bus_update(struct clk *bus)
+{
+	struct clk *c;
+	unsigned long rate = bus->min_rate;
+
+	list_for_each_entry(c, &bus->shared_bus_list, u.shared_bus_user.node)
+		if (c->u.shared_bus_user.enabled)
+			rate = max(c->u.shared_bus_user.rate, rate);
+
+	if (rate == clk_get_rate_locked(bus))
+		return 0;
+
+	return clk_set_rate_locked(bus, rate);
+};
+
+static void tegra_clk_shared_bus_init(struct clk *c)
+{
+	unsigned long flags;
+
+	c->max_rate = c->parent->max_rate;
+	c->u.shared_bus_user.rate = c->parent->max_rate;
+	c->state = OFF;
+#ifdef CONFIG_DEBUG_FS
+	c->set = 1;
+#endif
+
+	spin_lock_irqsave(&c->parent->spinlock, flags);
+
+	list_add_tail(&c->u.shared_bus_user.node,
+		&c->parent->shared_bus_list);
+
+	spin_unlock_irqrestore(&c->parent->spinlock, flags);
+}
+
+static int tegra_clk_shared_bus_set_rate(struct clk *c, unsigned long rate)
+{
+	unsigned long flags;
+	int ret;
+
+	rate = clk_round_rate(c->parent, rate);
+	if (rate < 0)
+		return rate;
+
+	spin_lock_irqsave(&c->parent->spinlock, flags);
+
+	c->u.shared_bus_user.rate = rate;
+	ret = tegra_clk_shared_bus_update(c->parent);
+
+	spin_unlock_irqrestore(&c->parent->spinlock, flags);
+
+	return ret;
+}
+
+static long tegra_clk_shared_bus_round_rate(struct clk *c, unsigned long rate)
+{
+	return clk_round_rate(c->parent, rate);
+}
+
+static int tegra_clk_shared_bus_enable(struct clk *c)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&c->parent->spinlock, flags);
+
+	c->u.shared_bus_user.enabled = true;
+	ret = tegra_clk_shared_bus_update(c->parent);
+
+	spin_unlock_irqrestore(&c->parent->spinlock, flags);
+
+	return ret;
+}
+
+static void tegra_clk_shared_bus_disable(struct clk *c)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&c->parent->spinlock, flags);
+
+	c->u.shared_bus_user.enabled = false;
+	ret = tegra_clk_shared_bus_update(c->parent);
+	WARN_ON_ONCE(ret);
+
+	spin_unlock_irqrestore(&c->parent->spinlock, flags);
+}
+
+static struct clk_ops tegra_clk_shared_bus_ops = {
+	.init = tegra_clk_shared_bus_init,
+	.enable = tegra_clk_shared_bus_enable,
+	.disable = tegra_clk_shared_bus_disable,
+	.set_rate = tegra_clk_shared_bus_set_rate,
+	.round_rate = tegra_clk_shared_bus_round_rate,
+};
+
+
 /* Clock definitions */
 static struct clk tegra_clk_32k = {
 	.name = "clk_32k",
@@ -1858,6 +1962,17 @@ static struct clk_mux_sel mux_clk_32k[] = {
 		},					\
 	}
 
+#define SHARED_CLK(_name, _dev, _con, _parent)		\
+	{						\
+		.name      = _name,			\
+		.lookup    = {				\
+			.dev_id    = _dev,		\
+			.con_id    = _con,		\
+		},					\
+		.ops       = &tegra_clk_shared_bus_ops,	\
+		.parent = _parent,			\
+	}
+
 struct clk tegra_list_clks[] = {
 	PERIPH_CLK("rtc",	"rtc-tegra",		NULL,	4,	0,	32768,     mux_clk_32k,			PERIPH_NO_RESET),
 	PERIPH_CLK("timer",	"timer",		NULL,	5,	0,	26000000,  mux_clk_m,			0),
@@ -2001,6 +2116,7 @@ struct clk *tegra_ptr_clks[] = {
 static void tegra2_init_one_clock(struct clk *c)
 {
 	clk_init(c);
+	INIT_LIST_HEAD(&c->shared_bus_list);
 	if (!c->lookup.dev_id && !c->lookup.con_id)
 		c->lookup.con_id = c->name;
 	c->lookup.clk = c;
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH] arm: mach-at91: remove double-semicolons
From: Sylvain Calador @ 2011-02-19 21:45 UTC (permalink / raw)
  To: linux-arm-kernel

Remove double-semicolons.

Signed-off-by: Sylvain Calador <sylvain.calador@gmail.com>
---
 arch/arm/mach-at91/at91cap9_devices.c    |    2 +-
 arch/arm/mach-at91/at91sam9g45_devices.c |    2 +-
 arch/arm/mach-at91/at91sam9rl_devices.c  |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-at91/at91cap9_devices.c b/arch/arm/mach-at91/at91cap9_devices.c
index d1f775e..308ce7a 100644
--- a/arch/arm/mach-at91/at91cap9_devices.c
+++ b/arch/arm/mach-at91/at91cap9_devices.c
@@ -171,7 +171,7 @@ void __init at91_add_device_usba(struct usba_platform_data *data)
 	 */
 	usba_udc_data.pdata.vbus_pin = -EINVAL;
 	usba_udc_data.pdata.num_ep = ARRAY_SIZE(usba_udc_ep);
-	memcpy(usba_udc_data.ep, usba_udc_ep, sizeof(usba_udc_ep));;
+	memcpy(usba_udc_data.ep, usba_udc_ep, sizeof(usba_udc_ep));
 
 	if (data && data->vbus_pin > 0) {
 		at91_set_gpio_input(data->vbus_pin, 0);
diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index 1e8f275..5e9f8a4 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -256,7 +256,7 @@ void __init at91_add_device_usba(struct usba_platform_data *data)
 {
 	usba_udc_data.pdata.vbus_pin = -EINVAL;
 	usba_udc_data.pdata.num_ep = ARRAY_SIZE(usba_udc_ep);
-	memcpy(usba_udc_data.ep, usba_udc_ep, sizeof(usba_udc_ep));;
+	memcpy(usba_udc_data.ep, usba_udc_ep, sizeof(usba_udc_ep));
 
 	if (data && data->vbus_pin > 0) {
 		at91_set_gpio_input(data->vbus_pin, 0);
diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
index 53aaa94..c49262b 100644
--- a/arch/arm/mach-at91/at91sam9rl_devices.c
+++ b/arch/arm/mach-at91/at91sam9rl_devices.c
@@ -145,7 +145,7 @@ void __init at91_add_device_usba(struct usba_platform_data *data)
 	 */
 	usba_udc_data.pdata.vbus_pin = -EINVAL;
 	usba_udc_data.pdata.num_ep = ARRAY_SIZE(usba_udc_ep);
-	memcpy(usba_udc_data.ep, usba_udc_ep, sizeof(usba_udc_ep));;
+	memcpy(usba_udc_data.ep, usba_udc_ep, sizeof(usba_udc_ep));
 
 	if (data && data->vbus_pin > 0) {
 		at91_set_gpio_input(data->vbus_pin, 0);
-- 
1.7.0.4

^ permalink raw reply related

* [GIT PULL] ARM: PMU for U8500
From: Linus Walleij @ 2011-02-19 21:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTikie8ZsTAv3fkS0AcVXpKhS3r8U8idajP3U=Mz8@mail.gmail.com>

2011/2/17 Linus Walleij <linus.walleij@linaro.org>:

> Russell, could you please pull:
>
> ? ? git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git
> pmu-for-rmk
>
> This is two patches giving us PMU support on the weird
> single-PMU-irq U8500.
>
> This is based on 2.6.38-rc4 and is identical to the v2
> patchset which was Acked by Will Deacon.
>
> Rabin Vincent (2):
> ?ARM: perf_event: allow platform-specific interrupt handler
> ?mach-ux500: DB8500 PMU support

I don't know why I missed to include the mailing list on this
one. :-(

I've included both patches in ux500-core so we get some -next
testing, at the begining of the patch series so they can be pulled
into your tree without any conflicts in -next if you decide to do
so.

Should be OK, but if the first patch disturbs anything in the
ARM tree merge with -next due to something else messing
around with the PMU files I will happily take it out again.

Yours,
Linus Walleij

^ permalink raw reply

* Question About Linux Memory Mapping
From: Marek Vasut @ 2011-02-19 20:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <AANLkTinUoFzzx4hM8SpUt-mpjn1Z5xmrDqmXwuELuu3e@mail.gmail.com>

On Friday 18 February 2011 21:56:28 Drasko DRASKOVIC wrote:
> Hi all,
> in the article Booting ARM Linux :
> http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html
> I can see that mem map is passed via ATAG_MEM. However, in the same
> article it is mentioned that this information can also be passed via
> kernel command line paramters,  mem=<size>[KM][,@<phys_offset>].
> 
> However, this does not seem to be true, as "mem" command line
> parameter seems to be formated like this : mem= n[KMG] (i.e. no
> offset), regarding to this reference :
> http://oreilly.com/linux/excerpts/9780596100797/kernel-boot-command-line-pa
> rameter-reference.html. Seems like memmap should be used instead.
> 
> I tried passing the parameters like memmap= n[KMG]@start[KMG] but that
> had no effect at all - still the same amount of System Ram was read
> from ATAGS and presented in the system via /proc/iomem.
> 
> What I needed it to reserve 1MB region for one FIFO at the end of RAM
> (or somewhere else)
> and protect it from the kernel. I tried passing memmap=
> n[KMG]$start[KMG], but that did not worn neither.

What are you exactly trying to achieve ? btw. if you really need to make a hole 
in RAM, you should reserve a bootmem node maybe?
> 
> So my questions are following :
> 1) Why commandlines are ignored and ATAGS are given priority ?
> 2) What is the most elegant way to protect one region in RAM :
>  a) By giving less memory with ATAGS_MEM and thus making protected
> region invisible to Linux, lying to it that RAM is smaller
>  b) By changing somehow linker script
>  c) By changing some configuration variables (which ?)
> 
> Thanks for the answers and best regards,
> Drasko
> 
> _______________________________________________
> linux-arm mailing list
> linux-arm at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm

^ permalink raw reply

* [PATCH 2/2] pl011: factor our FIFO to TTY code
From: Linus Walleij @ 2011-02-19 20:17 UTC (permalink / raw)
  To: linux-arm-kernel

From: Linus Walleij <linus.walleij@linaro.org>

This piece of code was just slightly different between the DMA
and IRQ paths, in DMA mode we surely shouldn't read more than
256 character either, so factor this out in its own function and
use for both DMA and PIO mode.

Tested on Ux500 and U300.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/tty/serial/amba-pl011.c |  157 ++++++++++++++++----------------------
 1 files changed, 66 insertions(+), 91 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index cb45136..faa16ee 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -144,6 +144,62 @@ struct uart_amba_port {
 };
 
 /*
+ * Reads up to 256 characters from the FIFO or until it's empty and
+ * inserts them into the TTY layer. Returns the number of characters
+ * read from the FIFO.
+ */
+static int pl011_fifo_to_tty(struct uart_amba_port *uap)
+{
+	u16 status, ch;
+	unsigned int flag, max_count = 256;
+	int fifotaken = 0;
+
+	while (max_count--) {
+		status = readw(uap->port.membase + UART01x_FR);
+		if (status & UART01x_FR_RXFE)
+			break;
+
+		/* Take chars from the FIFO and update status */
+		ch = readw(uap->port.membase + UART01x_DR) |
+			UART_DUMMY_DR_RX;
+		flag = TTY_NORMAL;
+		uap->port.icount.rx++;
+		fifotaken++;
+
+		if (unlikely(ch & UART_DR_ERROR)) {
+			if (ch & UART011_DR_BE) {
+				ch &= ~(UART011_DR_FE | UART011_DR_PE);
+				uap->port.icount.brk++;
+				if (uart_handle_break(&uap->port))
+					continue;
+			} else if (ch & UART011_DR_PE)
+				uap->port.icount.parity++;
+			else if (ch & UART011_DR_FE)
+				uap->port.icount.frame++;
+			if (ch & UART011_DR_OE)
+				uap->port.icount.overrun++;
+
+			ch &= uap->port.read_status_mask;
+
+			if (ch & UART011_DR_BE)
+				flag = TTY_BREAK;
+			else if (ch & UART011_DR_PE)
+				flag = TTY_PARITY;
+			else if (ch & UART011_DR_FE)
+				flag = TTY_FRAME;
+		}
+
+		if (uart_handle_sysrq_char(&uap->port, ch & 255))
+			continue;
+
+		uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
+	}
+
+	return fifotaken;
+}
+
+
+/*
  * All the DMA operation mode stuff goes inside this ifdef.
  * This assumes that you have a generic DMA device interface,
  * no custom DMA interfaces are supported.
@@ -634,7 +690,6 @@ static void pl011_dma_rx_chars(struct uart_amba_port *uap,
 	struct pl011_sgbuf *sgbuf = use_buf_b ?
 		&uap->dmarx.sgbuf_b : &uap->dmarx.sgbuf_a;
 	struct device *dev = uap->dmarx.chan->device->dev;
-	unsigned int status, ch, flag;
 	int dma_count = 0;
 	u32 fifotaken = 0; /* only used for vdbg() */
 
@@ -671,56 +726,16 @@ static void pl011_dma_rx_chars(struct uart_amba_port *uap,
 
 		/*
 		 * If we read all the DMA'd characters, and we had an
-		 * incomplete buffer, that could be due to an rx error,
-		 * or maybe we just timed out. Read any pending chars
-		 * and check the error status.
+		 * incomplete buffer, that could be due to an rx error, or
+		 * maybe we just timed out. Read any pending chars and check
+		 * the error status.
+		 *
+		 * Error conditions will only occur in the FIFO, these will
+		 * trigger an immediate interrupt and stop the DMA job, so we
+		 * will always find the error in the FIFO, never in the DMA
+		 * buffer.
 		 */
-		while (1) {
-			status = readw(uap->port.membase + UART01x_FR);
-			if (status & UART01x_FR_RXFE)
-				break;
-
-			/* Take chars from the FIFO and update status */
-			ch = readw(uap->port.membase + UART01x_DR) |
-			       UART_DUMMY_DR_RX;
-			flag = TTY_NORMAL;
-			uap->port.icount.rx++;
-			fifotaken++;
-
-			/*
-			 * Error conditions will only occur in the FIFO,
-			 * these will trigger an immediate interrupt and
-			 * stop the DMA job, so we will always find the
-			 * error in the FIFO, never in the DMA buffer.
-			 */
-			if (unlikely(ch & UART_DR_ERROR)) {
-				if (ch & UART011_DR_BE) {
-					ch &= ~(UART011_DR_FE | UART011_DR_PE);
-					uap->port.icount.brk++;
-					if (uart_handle_break(&uap->port))
-						continue;
-				} else if (ch & UART011_DR_PE)
-					uap->port.icount.parity++;
-				else if (ch & UART011_DR_FE)
-					uap->port.icount.frame++;
-				if (ch & UART011_DR_OE)
-					uap->port.icount.overrun++;
-
-				ch &= uap->port.read_status_mask;
-
-				if (ch & UART011_DR_BE)
-					flag = TTY_BREAK;
-				else if (ch & UART011_DR_PE)
-					flag = TTY_PARITY;
-				else if (ch & UART011_DR_FE)
-					flag = TTY_FRAME;
-			}
-
-			if (uart_handle_sysrq_char(&uap->port, ch & 255))
-				continue;
-
-			uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
-		}
+		fifotaken = pl011_fifo_to_tty(uap);
 	}
 
 	spin_unlock(&uap->port.lock);
@@ -1036,49 +1051,9 @@ static void pl011_enable_ms(struct uart_port *port)
 static void pl011_rx_chars(struct uart_amba_port *uap)
 {
 	struct tty_struct *tty = uap->port.state->port.tty;
-	unsigned int status, ch, flag, max_count = 256;
 
-	status = readw(uap->port.membase + UART01x_FR);
-	while ((status & UART01x_FR_RXFE) == 0 && max_count--) {
-		ch = readw(uap->port.membase + UART01x_DR) | UART_DUMMY_DR_RX;
-		flag = TTY_NORMAL;
-		uap->port.icount.rx++;
+	pl011_fifo_to_tty(uap);
 
-		/*
-		 * Note that the error handling code is
-		 * out of the main execution path
-		 */
-		if (unlikely(ch & UART_DR_ERROR)) {
-			if (ch & UART011_DR_BE) {
-				ch &= ~(UART011_DR_FE | UART011_DR_PE);
-				uap->port.icount.brk++;
-				if (uart_handle_break(&uap->port))
-					goto ignore_char;
-			} else if (ch & UART011_DR_PE)
-				uap->port.icount.parity++;
-			else if (ch & UART011_DR_FE)
-				uap->port.icount.frame++;
-			if (ch & UART011_DR_OE)
-				uap->port.icount.overrun++;
-
-			ch &= uap->port.read_status_mask;
-
-			if (ch & UART011_DR_BE)
-				flag = TTY_BREAK;
-			else if (ch & UART011_DR_PE)
-				flag = TTY_PARITY;
-			else if (ch & UART011_DR_FE)
-				flag = TTY_FRAME;
-		}
-
-		if (uart_handle_sysrq_char(&uap->port, ch & 255))
-			goto ignore_char;
-
-		uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
-
-	ignore_char:
-		status = readw(uap->port.membase + UART01x_FR);
-	}
 	spin_unlock(&uap->port.lock);
 	tty_flip_buffer_push(tty);
 	/*
-- 
1.7.3.2

^ permalink raw reply related

* [PATCH 1/2] pl011: add optional RX DMA to PL011 v2
From: Linus Walleij @ 2011-02-19 20:16 UTC (permalink / raw)
  To: linux-arm-kernel

From: Linus Walleij <linus.walleij@linaro.org>

This adds an optional RX DMA codepath for the devices that
support this by using the apropriate burst sizes instead of
pulling single bytes.

Includes portions of code written by Russell King during
a PL08x hacking session.

This has been tested on U300 and Ux500.

Tested-by: Jerzy Kasenberg <jerzy.kasenberg@tieto.com>
Tested-by: Grzegorz Sygieda <grzegorz.sygieda@tieto.com>
Tested-by: Marcin Mielczarczyk <marcin.mielczarczyk@tieto.com>
Signed-off-by: Per Forlin <per.friden@stericsson.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/tty/serial/amba-pl011.c |  454 +++++++++++++++++++++++++++++++++++++--
 1 files changed, 434 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index e76d7d0..cb45136 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -96,6 +96,22 @@ static struct vendor_data vendor_st = {
 };
 
 /* Deals with DMA transactions */
+
+struct pl011_sgbuf {
+	struct scatterlist sg;
+	char *buf;
+};
+
+struct pl011_dmarx_data {
+	struct dma_chan		*chan;
+	struct completion	complete;
+	bool			use_buf_b;
+	struct pl011_sgbuf	sgbuf_a;
+	struct pl011_sgbuf	sgbuf_b;
+	dma_cookie_t		cookie;
+	bool			running;
+};
+
 struct pl011_dmatx_data {
 	struct dma_chan		*chan;
 	struct scatterlist	sg;
@@ -120,7 +136,9 @@ struct uart_amba_port {
 	char			type[12];
 #ifdef CONFIG_DMA_ENGINE
 	/* DMA stuff */
-	bool			using_dma;
+	bool			using_tx_dma;
+	bool			using_rx_dma;
+	struct pl011_dmarx_data dmarx;
 	struct pl011_dmatx_data	dmatx;
 #endif
 };
@@ -134,6 +152,31 @@ struct uart_amba_port {
 
 #define PL011_DMA_BUFFER_SIZE PAGE_SIZE
 
+static int pl011_sgbuf_init(struct dma_chan *chan, struct pl011_sgbuf *sg,
+	enum dma_data_direction dir)
+{
+	sg->buf = kmalloc(PL011_DMA_BUFFER_SIZE, GFP_KERNEL);
+	if (!sg->buf)
+		return -ENOMEM;
+
+	sg_init_one(&sg->sg, sg->buf, PL011_DMA_BUFFER_SIZE);
+
+	if (dma_map_sg(chan->device->dev, &sg->sg, 1, dir) != 1) {
+		kfree(sg->buf);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static void pl011_sgbuf_free(struct dma_chan *chan, struct pl011_sgbuf *sg,
+	enum dma_data_direction dir)
+{
+	if (sg->buf) {
+		dma_unmap_sg(chan->device->dev, &sg->sg, 1, dir);
+		kfree(sg->buf);
+	}
+}
+
 static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 {
 	/* DMA is the sole user of the platform data right now */
@@ -153,7 +196,7 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 		return;
 	}
 
-	/* Try to acquire a generic DMA engine slave channel */
+	/* Try to acquire a generic DMA engine slave TX channel */
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
 
@@ -168,6 +211,28 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 
 	dev_info(uap->port.dev, "DMA channel TX %s\n",
 		 dma_chan_name(uap->dmatx.chan));
+
+	/* Optionally make use of an RX channel as well */
+	if (plat->dma_rx_param) {
+		struct dma_slave_config rx_conf = {
+			.src_addr = uap->port.mapbase + UART01x_DR,
+			.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
+			.direction = DMA_FROM_DEVICE,
+			.src_maxburst = uap->fifosize >> 1,
+		};
+
+		chan = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param);
+		if (!chan) {
+			dev_err(uap->port.dev, "no RX DMA channel!\n");
+			return;
+		}
+
+		dmaengine_slave_config(chan, &rx_conf);
+		uap->dmarx.chan = chan;
+
+		dev_info(uap->port.dev, "DMA channel RX %s\n",
+			 dma_chan_name(uap->dmarx.chan));
+	}
 }
 
 #ifndef MODULE
@@ -219,9 +284,10 @@ static void pl011_dma_remove(struct uart_amba_port *uap)
 	/* TODO: remove the initcall if it has not yet executed */
 	if (uap->dmatx.chan)
 		dma_release_channel(uap->dmatx.chan);
+	if (uap->dmarx.chan)
+		dma_release_channel(uap->dmarx.chan);
 }
 
-
 /* Forward declare this for the refill routine */
 static int pl011_dma_tx_refill(struct uart_amba_port *uap);
 
@@ -380,7 +446,7 @@ static int pl011_dma_tx_refill(struct uart_amba_port *uap)
  */
 static bool pl011_dma_tx_irq(struct uart_amba_port *uap)
 {
-	if (!uap->using_dma)
+	if (!uap->using_tx_dma)
 		return false;
 
 	/*
@@ -432,7 +498,7 @@ static inline bool pl011_dma_tx_start(struct uart_amba_port *uap)
 {
 	u16 dmacr;
 
-	if (!uap->using_dma)
+	if (!uap->using_tx_dma)
 		return false;
 
 	if (!uap->port.x_char) {
@@ -492,7 +558,7 @@ static void pl011_dma_flush_buffer(struct uart_port *port)
 {
 	struct uart_amba_port *uap = (struct uart_amba_port *)port;
 
-	if (!uap->using_dma)
+	if (!uap->using_tx_dma)
 		return;
 
 	/* Avoid deadlock with the DMA engine callback */
@@ -508,9 +574,260 @@ static void pl011_dma_flush_buffer(struct uart_port *port)
 	}
 }
 
+static void pl011_dma_rx_callback(void *data);
+
+static int pl011_dma_rx_trigger_dma(struct uart_amba_port *uap)
+{
+	struct dma_chan *rxchan = uap->dmarx.chan;
+	struct dma_device *dma_dev;
+	struct pl011_dmarx_data *dmarx = &uap->dmarx;
+	struct dma_async_tx_descriptor *desc;
+	struct pl011_sgbuf *sgbuf;
+
+	if (!rxchan)
+		return -EIO;
+
+	/* Start the RX DMA job */
+	sgbuf = uap->dmarx.use_buf_b ?
+		&uap->dmarx.sgbuf_b : &uap->dmarx.sgbuf_a;
+	dma_dev = rxchan->device;
+	desc = rxchan->device->device_prep_slave_sg(rxchan, &sgbuf->sg, 1,
+					DMA_FROM_DEVICE,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+	/*
+	 * If the DMA engine is busy and cannot prepare a
+	 * channel, no big deal, the driver will fall back
+	 * to interrupt mode as a result of this error code.
+	 */
+	if (!desc) {
+		uap->dmarx.running = false;
+		dmaengine_terminate_all(rxchan);
+		return -EBUSY;
+	}
+
+	/* Some data to go along to the callback */
+	desc->callback = pl011_dma_rx_callback;
+	desc->callback_param = uap;
+	dmarx->cookie = dmaengine_submit(desc);
+	dma_async_issue_pending(rxchan);
+
+	uap->dmacr |= UART011_RXDMAE;
+	writew(uap->dmacr, uap->port.membase + UART011_DMACR);
+	uap->dmarx.running = true;
+
+	uap->im &= ~UART011_RXIM;
+	writew(uap->im, uap->port.membase + UART011_IMSC);
+
+	return 0;
+}
+
+/*
+ * This is called when either the DMA job is complete, or
+ * the FIFO timeout interrupt occurred. This must be called
+ * with the port spinlock uap->port.lock held.
+ */
+static void pl011_dma_rx_chars(struct uart_amba_port *uap,
+			       u32 pending, bool use_buf_b,
+			       bool readfifo)
+{
+	struct tty_struct *tty = uap->port.state->port.tty;
+	struct pl011_sgbuf *sgbuf = use_buf_b ?
+		&uap->dmarx.sgbuf_b : &uap->dmarx.sgbuf_a;
+	struct device *dev = uap->dmarx.chan->device->dev;
+	unsigned int status, ch, flag;
+	int dma_count = 0;
+	u32 fifotaken = 0; /* only used for vdbg() */
+
+	/* Pick everything from the DMA first */
+	if (pending) {
+		/* Sync in buffer */
+		dma_sync_sg_for_cpu(dev, &sgbuf->sg, 1, DMA_FROM_DEVICE);
+
+		/*
+		 * First take all chars in the DMA pipe, then look in the FIFO.
+		 * Note that tty_insert_flip_buf() tries to take as many chars
+		 * as it can.
+		 */
+		dma_count = tty_insert_flip_string(uap->port.state->port.tty,
+						   sgbuf->buf, pending);
+
+		/* Return buffer to device */
+		dma_sync_sg_for_device(dev, &sgbuf->sg, 1, DMA_FROM_DEVICE);
+
+		uap->port.icount.rx += dma_count;
+		if (dma_count < pending)
+			dev_warn(uap->port.dev,
+				 "couldn't insert all characters (TTY is full?)\n");
+	}
+
+	/*
+	 * Only continue with trying to read the FIFO if all DMA chars have
+	 * been taken first.
+	 */
+	if (dma_count == pending && readfifo) {
+		/* Clear any error flags */
+		writew(UART011_OEIS | UART011_BEIS | UART011_PEIS | UART011_FEIS,
+		       uap->port.membase + UART011_ICR);
+
+		/*
+		 * If we read all the DMA'd characters, and we had an
+		 * incomplete buffer, that could be due to an rx error,
+		 * or maybe we just timed out. Read any pending chars
+		 * and check the error status.
+		 */
+		while (1) {
+			status = readw(uap->port.membase + UART01x_FR);
+			if (status & UART01x_FR_RXFE)
+				break;
+
+			/* Take chars from the FIFO and update status */
+			ch = readw(uap->port.membase + UART01x_DR) |
+			       UART_DUMMY_DR_RX;
+			flag = TTY_NORMAL;
+			uap->port.icount.rx++;
+			fifotaken++;
+
+			/*
+			 * Error conditions will only occur in the FIFO,
+			 * these will trigger an immediate interrupt and
+			 * stop the DMA job, so we will always find the
+			 * error in the FIFO, never in the DMA buffer.
+			 */
+			if (unlikely(ch & UART_DR_ERROR)) {
+				if (ch & UART011_DR_BE) {
+					ch &= ~(UART011_DR_FE | UART011_DR_PE);
+					uap->port.icount.brk++;
+					if (uart_handle_break(&uap->port))
+						continue;
+				} else if (ch & UART011_DR_PE)
+					uap->port.icount.parity++;
+				else if (ch & UART011_DR_FE)
+					uap->port.icount.frame++;
+				if (ch & UART011_DR_OE)
+					uap->port.icount.overrun++;
+
+				ch &= uap->port.read_status_mask;
+
+				if (ch & UART011_DR_BE)
+					flag = TTY_BREAK;
+				else if (ch & UART011_DR_PE)
+					flag = TTY_PARITY;
+				else if (ch & UART011_DR_FE)
+					flag = TTY_FRAME;
+			}
+
+			if (uart_handle_sysrq_char(&uap->port, ch & 255))
+				continue;
+
+			uart_insert_char(&uap->port, ch, UART011_DR_OE, ch, flag);
+		}
+	}
+
+	spin_unlock(&uap->port.lock);
+	dev_vdbg(uap->port.dev,
+		 "Took %d chars from DMA buffer and %d chars from the FIFO\n",
+		 dma_count, fifotaken);
+	tty_flip_buffer_push(tty);
+	spin_lock(&uap->port.lock);
+}
+
+static void pl011_dma_rx_irq(struct uart_amba_port *uap)
+{
+	struct pl011_dmarx_data *dmarx = &uap->dmarx;
+	struct dma_chan *rxchan = dmarx->chan;
+	struct pl011_sgbuf *sgbuf = dmarx->use_buf_b ?
+		&dmarx->sgbuf_b : &dmarx->sgbuf_a;
+	size_t pending;
+	struct dma_tx_state state;
+	enum dma_status dmastat;
+
+	/*
+	 * Pause the transfer so we can trust the current counter,
+	 * do this before we pause the PL011 block, else we may
+	 * overflow the FIFO.
+	 */
+	if (dmaengine_pause(rxchan))
+		dev_err(uap->port.dev, "unable to pause DMA transfer\n");
+	dmastat = rxchan->device->device_tx_status(rxchan,
+						   dmarx->cookie, &state);
+	if (dmastat != DMA_PAUSED)
+		dev_err(uap->port.dev, "unable to pause DMA transfer\n");
+
+	/* Disable RX DMA - incoming data will wait in the FIFO */
+	uap->dmacr &= ~UART011_RXDMAE;
+	writew(uap->dmacr, uap->port.membase + UART011_DMACR);
+	uap->dmarx.running = false;
+
+	pending = sgbuf->sg.length - state.residue;
+	BUG_ON(pending > PL011_DMA_BUFFER_SIZE);
+	/* Then we terminate the transfer - we now know our residue */
+	dmaengine_terminate_all(rxchan);
+
+	/*
+	 * This will take the chars we have so far and insert
+	 * into the framework.
+	 */
+	pl011_dma_rx_chars(uap, pending, dmarx->use_buf_b, true);
+
+	/* Switch buffer & re-trigger DMA job */
+	dmarx->use_buf_b = !dmarx->use_buf_b;
+	if (pl011_dma_rx_trigger_dma(uap)) {
+		dev_dbg(uap->port.dev, "could not retrigger RX DMA job "
+			"fall back to interrupt mode\n");
+		uap->im |= UART011_RXIM;
+		writew(uap->im, uap->port.membase + UART011_IMSC);
+	}
+}
+
+static void pl011_dma_rx_callback(void *data)
+{
+	struct uart_amba_port *uap = data;
+	struct pl011_dmarx_data *dmarx = &uap->dmarx;
+	bool lastbuf = dmarx->use_buf_b;
+	int ret;
+
+	/*
+	 * This completion interrupt occurs typically when the
+	 * RX buffer is totally stuffed but no timeout has yet
+	 * occurred. When that happens, we just want the RX
+	 * routine to flush out the secondary DMA buffer while
+	 * we immediately trigger the next DMA job.
+	 */
+	spin_lock_irq(&uap->port.lock);
+	uap->dmarx.running = false;
+	dmarx->use_buf_b = !lastbuf;
+	ret = pl011_dma_rx_trigger_dma(uap);
+
+	pl011_dma_rx_chars(uap, PL011_DMA_BUFFER_SIZE, lastbuf, false);
+	spin_unlock_irq(&uap->port.lock);
+	/*
+	 * Do this check after we picked the DMA chars so we don't
+	 * get some IRQ immediately from RX.
+	 */
+	if (ret) {
+		dev_dbg(uap->port.dev, "could not retrigger RX DMA job "
+			"fall back to interrupt mode\n");
+		uap->im |= UART011_RXIM;
+		writew(uap->im, uap->port.membase + UART011_IMSC);
+	}
+}
+
+/*
+ * Stop accepting received characters, when we're shutting down or
+ * suspending this port.
+ * Locking: called with port lock held and IRQs disabled.
+ */
+static inline void pl011_dma_rx_stop(struct uart_amba_port *uap)
+{
+	/* FIXME.  Just disable the DMA enable */
+	uap->dmacr &= ~UART011_RXDMAE;
+	writew(uap->dmacr, uap->port.membase + UART011_DMACR);
+}
 
 static void pl011_dma_startup(struct uart_amba_port *uap)
 {
+	int ret;
+
 	if (!uap->dmatx.chan)
 		return;
 
@@ -525,8 +842,33 @@ static void pl011_dma_startup(struct uart_amba_port *uap)
 
 	/* The DMA buffer is now the FIFO the TTY subsystem can use */
 	uap->port.fifosize = PL011_DMA_BUFFER_SIZE;
-	uap->using_dma = true;
+	uap->using_tx_dma = true;
+
+	if (!uap->dmarx.chan)
+		goto skip_rx;
+
+	/* Allocate and map DMA RX buffers */
+	ret = pl011_sgbuf_init(uap->dmarx.chan, &uap->dmarx.sgbuf_a,
+			       DMA_FROM_DEVICE);
+	if (ret) {
+		dev_err(uap->port.dev, "failed to init DMA %s: %d\n",
+			"RX buffer A", ret);
+		goto skip_rx;
+	}
+
+	ret = pl011_sgbuf_init(uap->dmarx.chan, &uap->dmarx.sgbuf_b,
+			       DMA_FROM_DEVICE);
+	if (ret) {
+		dev_err(uap->port.dev, "failed to init DMA %s: %d\n",
+			"RX buffer B", ret);
+		pl011_sgbuf_free(uap->dmarx.chan, &uap->dmarx.sgbuf_a,
+				 DMA_FROM_DEVICE);
+		goto skip_rx;
+	}
+
+	uap->using_rx_dma = true;
 
+skip_rx:
 	/* Turn on DMA error (RX/TX will be enabled on demand) */
 	uap->dmacr |= UART011_DMAONERR;
 	writew(uap->dmacr, uap->port.membase + UART011_DMACR);
@@ -539,11 +881,17 @@ static void pl011_dma_startup(struct uart_amba_port *uap)
 	if (uap->vendor->dma_threshold)
 		writew(ST_UART011_DMAWM_RX_16 | ST_UART011_DMAWM_TX_16,
 			       uap->port.membase + ST_UART011_DMAWM);
+
+	if (uap->using_rx_dma) {
+		if (pl011_dma_rx_trigger_dma(uap))
+			dev_dbg(uap->port.dev, "could not trigger initial "
+				"RX DMA job, fall back to interrupt mode\n");
+	}
 }
 
 static void pl011_dma_shutdown(struct uart_amba_port *uap)
 {
-	if (!uap->using_dma)
+	if (!(uap->using_tx_dma || uap->using_rx_dma))
 		return;
 
 	/* Disable RX and TX DMA */
@@ -555,19 +903,39 @@ static void pl011_dma_shutdown(struct uart_amba_port *uap)
 	writew(uap->dmacr, uap->port.membase + UART011_DMACR);
 	spin_unlock_irq(&uap->port.lock);
 
-	/* In theory, this should already be done by pl011_dma_flush_buffer */
-	dmaengine_terminate_all(uap->dmatx.chan);
-	if (uap->dmatx.queued) {
-		dma_unmap_sg(uap->dmatx.chan->device->dev, &uap->dmatx.sg, 1,
-			     DMA_TO_DEVICE);
-		uap->dmatx.queued = false;
+	if (uap->using_tx_dma) {
+		/* In theory, this should already be done by pl011_dma_flush_buffer */
+		dmaengine_terminate_all(uap->dmatx.chan);
+		if (uap->dmatx.queued) {
+			dma_unmap_sg(uap->dmatx.chan->device->dev, &uap->dmatx.sg, 1,
+				     DMA_TO_DEVICE);
+			uap->dmatx.queued = false;
+		}
+
+		kfree(uap->dmatx.buf);
+		uap->using_tx_dma = false;
+	}
+
+	if (uap->using_rx_dma) {
+		dmaengine_terminate_all(uap->dmarx.chan);
+		/* Clean up the RX DMA */
+		pl011_sgbuf_free(uap->dmarx.chan, &uap->dmarx.sgbuf_a, DMA_FROM_DEVICE);
+		pl011_sgbuf_free(uap->dmarx.chan, &uap->dmarx.sgbuf_b, DMA_FROM_DEVICE);
+		uap->using_rx_dma = false;
 	}
+}
 
-	kfree(uap->dmatx.buf);
+static inline bool pl011_dma_rx_available(struct uart_amba_port *uap)
+{
+	return uap->using_rx_dma;
+}
 
-	uap->using_dma = false;
+static inline bool pl011_dma_rx_running(struct uart_amba_port *uap)
+{
+	return uap->using_rx_dma && uap->dmarx.running;
 }
 
+
 #else
 /* Blank functions if the DMA engine is not available */
 static inline void pl011_dma_probe(struct uart_amba_port *uap)
@@ -600,6 +968,29 @@ static inline bool pl011_dma_tx_start(struct uart_amba_port *uap)
 	return false;
 }
 
+static inline void pl011_dma_rx_irq(struct uart_amba_port *uap)
+{
+}
+
+static inline void pl011_dma_rx_stop(struct uart_amba_port *uap)
+{
+}
+
+static inline int pl011_dma_rx_trigger_dma(struct uart_amba_port *uap)
+{
+	return -EIO;
+}
+
+static inline bool pl011_dma_rx_available(struct uart_amba_port *uap)
+{
+	return false;
+}
+
+static inline bool pl011_dma_rx_running(struct uart_amba_port *uap)
+{
+	return false;
+}
+
 #define pl011_dma_flush_buffer	NULL
 #endif
 
@@ -630,6 +1021,8 @@ static void pl011_stop_rx(struct uart_port *port)
 	uap->im &= ~(UART011_RXIM|UART011_RTIM|UART011_FEIM|
 		     UART011_PEIM|UART011_BEIM|UART011_OEIM);
 	writew(uap->im, uap->port.membase + UART011_IMSC);
+
+	pl011_dma_rx_stop(uap);
 }
 
 static void pl011_enable_ms(struct uart_port *port)
@@ -688,6 +1081,19 @@ static void pl011_rx_chars(struct uart_amba_port *uap)
 	}
 	spin_unlock(&uap->port.lock);
 	tty_flip_buffer_push(tty);
+	/*
+	 * If we were temporarily out of DMA mode for a while,
+	 * attempt to switch back to DMA mode again.
+	 */
+	if (pl011_dma_rx_available(uap)) {
+		if (pl011_dma_rx_trigger_dma(uap)) {
+			dev_dbg(uap->port.dev, "could not trigger RX DMA job "
+				"fall back to interrupt mode again\n");
+			uap->im |= UART011_RXIM;
+		} else
+			uap->im &= ~UART011_RXIM;
+		writew(uap->im, uap->port.membase + UART011_IMSC);
+	}
 	spin_lock(&uap->port.lock);
 }
 
@@ -767,8 +1173,12 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
 					  UART011_RXIS),
 			       uap->port.membase + UART011_ICR);
 
-			if (status & (UART011_RTIS|UART011_RXIS))
-				pl011_rx_chars(uap);
+			if (status & (UART011_RTIS|UART011_RXIS)) {
+				if (pl011_dma_rx_running(uap))
+					pl011_dma_rx_irq(uap);
+				else
+					pl011_rx_chars(uap);
+			}
 			if (status & (UART011_DSRMIS|UART011_DCDMIS|
 				      UART011_CTSMIS|UART011_RIMIS))
 				pl011_modem_status(uap);
@@ -945,10 +1355,14 @@ static int pl011_startup(struct uart_port *port)
 	pl011_dma_startup(uap);
 
 	/*
-	 * Finally, enable interrupts
+	 * Finally, enable interrupts, only timeouts when using DMA
+	 * if initial RX DMA job failed, start in interrupt mode
+	 * as well.
 	 */
 	spin_lock_irq(&uap->port.lock);
-	uap->im = UART011_RXIM | UART011_RTIM;
+	uap->im = UART011_RTIM;
+	if (!pl011_dma_rx_running(uap))
+		uap->im |= UART011_RXIM;
 	writew(uap->im, uap->port.membase + UART011_IMSC);
 	spin_unlock_irq(&uap->port.lock);
 
-- 
1.7.3.2

^ permalink raw reply related

* [PATCH] perf: add OMAP support for the new power events
From: Santosh Shilimkar @ 2011-02-19 18:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298052645-4164-1-git-send-email-j-pihet@ti.com>

> -----Original Message-----
> From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-
> owner at vger.kernel.org] On Behalf Of jean.pihet at newoldbits.com
> Sent: Friday, February 18, 2011 11:41 PM
> To: Kevin Hilman; Thomas Renninger; linux-omap at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org
> Cc: Jean Pihet
> Subject: [PATCH] perf: add OMAP support for the new power events
>
> From: Jean Pihet <j-pihet@ti.com>
>
> The patch adds the new power management trace points for
> the OMAP architecture.
>
> The trace points are for:
> - default idle handler. Since the cpuidle framework is
>   instrumented in the generic way there is no need to
>   add trace points in the OMAP specific cpuidle handler;
> - cpufreq (DVFS),
> - SoC clocks changes (enable, disable, set_rate),
> - power domain states: the desired target state and -if different-
>   the actually hit state.
>
> Because of the generic nature of the changes, OMAP3 and OMAP4 are
> supported.
>
> Tested on OMAP3 with suspend/resume, cpuidle, basic DVFS.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/clock.c       |    8 +++++++-
>  arch/arm/mach-omap2/pm34xx.c      |    7 +++++++
>  arch/arm/mach-omap2/powerdomain.c |   26 +++++++++++++++++++++++---
>  3 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-
> omap2/clock.c
> index 2a2f152..72af75d 100644
> --- a/arch/arm/mach-omap2/clock.c
> +++ b/arch/arm/mach-omap2/clock.c
> @@ -22,7 +22,9 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/bitops.h>
> +#include <trace/events/power.h>
>
> +#include <asm/cpu.h>
>  #include <plat/clock.h>
>  #include "clockdomain.h"
>  #include <plat/cpu.h>
> @@ -261,6 +263,7 @@ void omap2_clk_disable(struct clk *clk)
>
>  	pr_debug("clock: %s: disabling in hardware\n", clk->name);
>
> +	trace_clock_disable(clk->name, 0, smp_processor_id());
>  	clk->ops->disable(clk);
>
>  	if (clk->clkdm)
> @@ -312,6 +315,7 @@ int omap2_clk_enable(struct clk *clk)
>  		}
>  	}
>
> +	trace_clock_enable(clk->name, 1, smp_processor_id());
>  	ret = clk->ops->enable(clk);
>  	if (ret) {
>  		WARN(1, "clock: %s: could not enable: %d\n", clk->name,
> ret);
> @@ -349,8 +353,10 @@ int omap2_clk_set_rate(struct clk *clk,
> unsigned long rate)
>  	pr_debug("clock: set_rate for clock %s to rate %ld\n", clk-
> >name, rate);
>
>  	/* dpll_ck, core_ck, virt_prcm_set; plus all clksel clocks */
> -	if (clk->set_rate)
> +	if (clk->set_rate) {
> +		trace_clock_set_rate(clk->name, rate,
> smp_processor_id());
>  		ret = clk->set_rate(clk, rate);
> +	}
>
>  	return ret;
>  }
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-
> omap2/pm34xx.c
> index 2f864e4..d1cc3f4 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -29,6 +29,7 @@
>  #include <linux/delay.h>
>  #include <linux/slab.h>
>  #include <linux/console.h>
> +#include <trace/events/power.h>
>
>  #include <plat/sram.h>
>  #include "clockdomain.h"
> @@ -519,8 +520,14 @@ static void omap3_pm_idle(void)
>  	if (omap_irq_pending() || need_resched())
>  		goto out;
>
> +	trace_power_start(POWER_CSTATE, 1, smp_processor_id());
> +	trace_cpu_idle(1, smp_processor_id());
> +

This default idle code won't be used when you enable the
CONFIG_CPUIDLE. That case the cpuidle34xx.c idle code gets
registered.

Shouldn't you patch that code instead? This is more or less
dead code and it is just like default idle code when idle
drivers isn't registered.


>  	omap_sram_idle();
>
> +	trace_power_end(smp_processor_id());
> +	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
> +
>  out:
>  	local_fiq_enable();
>  	local_irq_enable();
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-
> omap2/powerdomain.c
> index eaed0df..1495eed 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -19,12 +19,15 @@
>  #include <linux/list.h>
>  #include <linux/errno.h>
>  #include <linux/string.h>
> +#include <trace/events/power.h>
> +
>  #include "cm2xxx_3xxx.h"
>  #include "prcm44xx.h"
>  #include "cm44xx.h"
>  #include "prm2xxx_3xxx.h"
>  #include "prm44xx.h"
>
> +#include <asm/cpu.h>
>  #include <plat/cpu.h>
>  #include "powerdomain.h"
>  #include "clockdomain.h"
> @@ -32,6 +35,8 @@
>
>  #include "pm.h"
>
> +#define PWRDM_TRACE_STATES_FLAG	(1<<31)
> +
>  enum {
>  	PWRDM_STATE_NOW = 0,
>  	PWRDM_STATE_PREV,
> @@ -130,8 +135,7 @@ static void
> _update_logic_membank_counters(struct powerdomain *pwrdm)
>  static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
>  {
>
> -	int prev;
> -	int state;
> +	int prev, state, trace_state = 0;
>
>  	if (pwrdm == NULL)
>  		return -EINVAL;
> @@ -148,6 +152,17 @@ static int _pwrdm_state_switch(struct
> powerdomain *pwrdm, int flag)
>  			pwrdm->state_counter[prev]++;
>  		if (prev == PWRDM_POWER_RET)
>  			_update_logic_membank_counters(pwrdm);
> +		/*
> +		 * If the power domain did not hit the desired state,
> +		 * generate a trace event with both the desired and hit
> states
> +		 */
> +		if (state != prev) {
> +			trace_state = (PWRDM_TRACE_STATES_FLAG |
> +				       ((state & OMAP_POWERSTATE_MASK) <<
8)
> |
> +				       ((prev & OMAP_POWERSTATE_MASK) <<
> 0));
> +			trace_power_domain_target(pwrdm->name,
> trace_state,
> +						  smp_processor_id());
> +		}
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -406,8 +421,13 @@ int pwrdm_set_next_pwrst(struct powerdomain
> *pwrdm, u8 pwrst)
>  	pr_debug("powerdomain: setting next powerstate for %s to
> %0x\n",
>  		 pwrdm->name, pwrst);
>
> -	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst)
> +	if (arch_pwrdm && arch_pwrdm->pwrdm_set_next_pwrst) {
> +		/* Trace the pwrdm desired target state */
> +		trace_power_domain_target(pwrdm->name, pwrst,
> +					  smp_processor_id());
> +		/* Program the pwrdm desired target state */
>  		ret = arch_pwrdm->pwrdm_set_next_pwrst(pwrdm, pwrst);
> +	}
>
>  	return ret;
>  }
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 1/2] PRUSS UIO driver support
From: Hans J. Koch @ 2011-02-19 18:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <B85A65D85D7EB246BE421B3FB0FBB593024B83C108@dbde02.ent.ti.com>

On Sat, Feb 19, 2011 at 09:10:23PM +0530, TK, Pratheesh Gangadhar wrote:
> 
> For my understanding - if the interrupt is not shared and not level triggered -
> is this okay to have empty handler?

Greg already said he won't accept that. And I'm quite sure these interrupts
are level triggered, since that is the default on arch/omap.

E.g. in arch/arm/mach-omap1/irq.c, a loop sets all irqs to level triggered
handling: set_irq_handler(j, handle_level_irq); (line 234)

> In this specific case, these interrupt lines are internal to SOC and hooked to ARM INTC from PRUSS. PRUSS has another INTC to handle system events to PRUSS as well as to generate system events to host ARM. These generated events are used for IPC between user application and PRU firmware and for async notifications from PRU firmware to user space. I don't see a reason to make it shared as we have 8 lines available for use. As mentioned before ARM INTC interrupt processing logic converts interrupts to active high pulses.

What's a "pulse triggered interrupt"? I know level and edge triggered ones.

> 
> I also looked at the interrupt handling in existing UIO drivers
> 
> 
> static irqreturn_t my_uio_handler(int irq, struct uio_info *dev_info)
> {
>         if (no interrupt is enabled and no interrupt is active) /For shared interrupt handling
>                 return IRQ_NONE;
> 
>         disable interrupt; // For level triggered interrupts 
>         return IRQ_HANDLED;
> }
> 
> It's not clear how and where interrupts are re-enabled. Is this expected to be done from user space?

That's the normal case, yes. You re-enable the interrupts by accessing the irq
mask register of your chip.

> 
> Uio_secos3.c has an irqcontrol function to enable/disable interrupts. Is this the recommended approach?

No. That is a workaround for broken hardware. You need it if and only if your
chip 1) has several internal irq sources and 2) there's no possibility for
the kernel to mask the interrupt in the chip without loosing the information
which irq source actually triggered the interrupt.

If that is the case, the kernel will disable the irq line in some other way
without touching the chip. Userspace then needs a way to re-enable irqs,
and that's where the irqcontrol function is used.

Normally, you shouldn't need it.

Thanks,
Hans

^ permalink raw reply

* [PATCH v4 12/19] ARM: LPAE: Add context switching support
From: Russell King - ARM Linux @ 2011-02-19 18:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1297689846.31111.43.camel@e102109-lin.cambridge.arm.com>

On Mon, Feb 14, 2011 at 01:24:06PM +0000, Catalin Marinas wrote:
> On Sat, 2011-02-12 at 10:44 +0000, Russell King - ARM Linux wrote:
> > On Mon, Jan 24, 2011 at 05:55:54PM +0000, Catalin Marinas wrote:
> > > +#ifdef CONFIG_ARM_LPAE
> > > +#define cpu_set_asid(asid) {                                         \
> > > +     unsigned long ttbl, ttbh;                                       \
> > > +     asm("   mrrc    p15, 0, %0, %1, c2              @ read TTBR0\n" \
> > > +         "   mov     %1, %1, lsl #(48 - 32)          @ set ASID\n"   \
> > > +         "   mcrr    p15, 0, %0, %1, c2              @ set TTBR0\n"  \
> > > +         : "=r" (ttbl), "=r" (ttbh)                                  \
> > > +         : "r" (asid & ~ASID_MASK));                                 \
> > 
> > This is wrong:
> > 1. It does nothing with %2 (the new asid)
> > 2. it shifts the high address bits of TTBR0 left 16 places each time its
> >    called.
> 
> It was worse actually, not even compiled in because it had output
> arguments but it wasn't volatile. Some early clobber is also needed.
> What about this:
> 
> #define cpu_set_asid(asid) {						\
> 	unsigned long ttbl, ttbh;					\
> 	asm volatile(							\
> 	"	mrrc	p15, 0, %0, %1, c2		@ read TTBR0\n"	\
> 	"	mov	%1, %2, lsl #(48 - 32)		@ set ASID\n"	\
> 	"	mcrr	p15, 0, %0, %1, c2		@ set TTBR0\n"	\
> 	: "=&r" (ttbl), "=&r" (ttbh)					\
> 	: "r" (asid & ~ASID_MASK));					\
> }

So we don't care about the low 16 bits of ttbh which can be simply zeroed?

^ permalink raw reply

* [PATCH v4 15/19] ARM: LPAE: use phys_addr_t instead of unsigned long for physical addresses
From: Russell King - ARM Linux @ 2011-02-19 18:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1295891761-18366-16-git-send-email-catalin.marinas@arm.com>

On Mon, Jan 24, 2011 at 05:55:57PM +0000, Catalin Marinas wrote:
> From: Will Deacon <will.deacon@arm.com>
> 
> The unsigned long datatype is not sufficient for mapping physical addresses
> >= 4GB.
> 
> This patch ensures that the phys_addr_t datatype is used to represent
> physical addresses which may be beyond the range of an unsigned long.
> The virt <-> phys macros are updated accordingly to ensure that virtual
> addresses can remain as they are.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

This patch needs some more things fixed to prevent warnings:

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index a81355d..6cf76b3 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -809,9 +809,10 @@ static void __init sanity_check_meminfo(void)
 		 */
 		if (__va(bank->start) >= vmalloc_min ||
 		    __va(bank->start) < (void *)PAGE_OFFSET) {
-			printk(KERN_NOTICE "Ignoring RAM at %.8lx-%.8lx "
+			printk(KERN_NOTICE "Ignoring RAM@%.8llx-%.8llx "
 			       "(vmalloc region overlap).\n",
-			       bank->start, bank->start + bank->size - 1);
+			       (unsigned long long)bank->start,
+			       (unsigned long long)bank->start + bank->size - 1);
 			continue;
 		}
 
@@ -822,10 +823,11 @@ static void __init sanity_check_meminfo(void)
 		if (__va(bank->start + bank->size) > vmalloc_min ||
 		    __va(bank->start + bank->size) < __va(bank->start)) {
 			unsigned long newsize = vmalloc_min - __va(bank->start);
-			printk(KERN_NOTICE "Truncating RAM at %.8lx-%.8lx "
-			       "to -%.8lx (vmalloc region overlap).\n",
-			       bank->start, bank->start + bank->size - 1,
-			       bank->start + newsize - 1);
+			printk(KERN_NOTICE "Truncating RAM at %.8llx-%.8llx "
+			       "to -%.8llx (vmalloc region overlap).\n",
+			       (unsigned long long)bank->start,
+			       (unsigned long long)bank->start + bank->size - 1,
+			       (unsigned long long)bank->start + newsize - 1);
 			bank->size = newsize;
 		}
 #endif

^ permalink raw reply related

* [PATCH V5 30/63] ST SPEAr: Replacing SIZE macro's with actual required size
From: Russell King - ARM Linux @ 2011-02-19 18:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <38be663ab3e653d889d64005a86ed4dcf8661ae1.1295499395.git.viresh.kumar@st.com>

Same comments as the other patch changing to SZ_* macros.  Where possible
fold these changes into the patch(es) which add the original definitions
rather than having patches which add stuff and then separate patches
which rework those patches.

Could you do that and provide reworked patches please?  I'll publish the
branch I've merged stuff onto thus far (but note that it's 'unstable'),
but note that it won't contain the bits I've merged into the 'fixes'
branch.

On Thu, Jan 20, 2011 at 12:56:08PM +0530, Viresh Kumar wrote:
> From: Shiraz Hashim <shiraz.hashim@st.com>
> 
> In arch specific files SIZE macro's were defined which were used for
> creating memory/io mappings for sepecific devices. These macro's are
> coming straight from h/w user manual and are much greated than required
> sizes. Replacing these macros by actual required size.
> 
> Signed-off-by: Shiraz Hashim <shiraz.hashim@st.com>
> Signed-off-by: Rajeev Kumar <rajeev-dlh.kumar@st.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  arch/arm/mach-spear3xx/spear300.c |    9 +++------
>  arch/arm/mach-spear3xx/spear310.c |    2 +-
>  arch/arm/mach-spear3xx/spear320.c |    2 +-
>  arch/arm/mach-spear3xx/spear3xx.c |    8 ++++----
>  arch/arm/mach-spear6xx/spear6xx.c |   10 +++++-----
>  5 files changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/mach-spear3xx/spear300.c b/arch/arm/mach-spear3xx/spear300.c
> index 6989407..30e893f 100644
> --- a/arch/arm/mach-spear3xx/spear300.c
> +++ b/arch/arm/mach-spear3xx/spear300.c
> @@ -604,8 +604,7 @@ struct spear_shirq shirq_ras1 = {
>  void sdhci_i2s_mem_enable(u8 mask)
>  {
>  	u32 val;
> -	void __iomem *base = ioremap(SPEAR300_SOC_CONFIG_BASE,
> -			SPEAR300_SOC_CONFIG_SIZE);
> +	void __iomem *base = ioremap(SPEAR300_SOC_CONFIG_BASE, SZ_4K);
>  	if (!base) {
>  		pr_debug("sdhci_i2s_enb: ioremap fail\n");
>  		return;
> @@ -628,8 +627,7 @@ void __init spear300_init(void)
>  	spear3xx_init();
>  
>  	/* shared irq registration */
> -	shirq_ras1.regs.base =
> -		ioremap(SPEAR300_TELECOM_BASE, SPEAR300_TELECOM_REG_SIZE);
> +	shirq_ras1.regs.base = ioremap(SPEAR300_TELECOM_BASE, SZ_4K);
>  	if (shirq_ras1.regs.base) {
>  		ret = spear_shirq_register(&shirq_ras1);
>  		if (ret)
> @@ -637,8 +635,7 @@ void __init spear300_init(void)
>  	}
>  
>  	/* pmx initialization */
> -	pmx_driver.base = ioremap(SPEAR300_SOC_CONFIG_BASE,
> -			SPEAR300_SOC_CONFIG_SIZE);
> +	pmx_driver.base = ioremap(SPEAR300_SOC_CONFIG_BASE, SZ_4K);
>  	if (pmx_driver.base) {
>  		ret = pmx_register(&pmx_driver);
>  		if (ret)
> diff --git a/arch/arm/mach-spear3xx/spear310.c b/arch/arm/mach-spear3xx/spear310.c
> index bb9ff7c..ffc1f63 100644
> --- a/arch/arm/mach-spear3xx/spear310.c
> +++ b/arch/arm/mach-spear3xx/spear310.c
> @@ -362,7 +362,7 @@ void __init spear310_init(void)
>  	spear3xx_init();
>  
>  	/* shared irq registration */
> -	base = ioremap(SPEAR310_SOC_CONFIG_BASE, SPEAR310_SOC_CONFIG_SIZE);
> +	base = ioremap(SPEAR310_SOC_CONFIG_BASE, SZ_4K);
>  	if (base) {
>  		/* shirq 1 */
>  		shirq_ras1.regs.base = base;
> diff --git a/arch/arm/mach-spear3xx/spear320.c b/arch/arm/mach-spear3xx/spear320.c
> index ab4ad8e..ac4f80b 100644
> --- a/arch/arm/mach-spear3xx/spear320.c
> +++ b/arch/arm/mach-spear3xx/spear320.c
> @@ -694,7 +694,7 @@ void __init spear320_init(void)
>  	spear3xx_init();
>  
>  	/* shared irq registration */
> -	base = ioremap(SPEAR320_SOC_CONFIG_BASE, SPEAR320_SOC_CONFIG_SIZE);
> +	base = ioremap(SPEAR320_SOC_CONFIG_BASE, SZ_4K);
>  	if (base) {
>  		/* shirq 1 */
>  		shirq_ras1.regs.base = base;
> diff --git a/arch/arm/mach-spear3xx/spear3xx.c b/arch/arm/mach-spear3xx/spear3xx.c
> index 6be49b3..0088b7c 100644
> --- a/arch/arm/mach-spear3xx/spear3xx.c
> +++ b/arch/arm/mach-spear3xx/spear3xx.c
> @@ -235,22 +235,22 @@ struct map_desc spear3xx_io_desc[] __initdata = {
>  	{
>  		.virtual	= VA_SPEAR3XX_ICM1_UART_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR3XX_ICM1_UART_BASE),
> -		.length		= SPEAR3XX_ICM1_UART_SIZE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	}, {
>  		.virtual	= VA_SPEAR3XX_ML1_VIC_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR3XX_ML1_VIC_BASE),
> -		.length		= SPEAR3XX_ML1_VIC_SIZE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	}, {
>  		.virtual	= VA_SPEAR3XX_ICM3_SYS_CTRL_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR3XX_ICM3_SYS_CTRL_BASE),
> -		.length		= SPEAR3XX_ICM3_SYS_CTRL_SIZE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	}, {
>  		.virtual	= VA_SPEAR3XX_ICM3_MISC_REG_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR3XX_ICM3_MISC_REG_BASE),
> -		.length		= SPEAR3XX_ICM3_MISC_REG_SIZE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	},
>  };
> diff --git a/arch/arm/mach-spear6xx/spear6xx.c b/arch/arm/mach-spear6xx/spear6xx.c
> index a948dd4..f8be93a 100644
> --- a/arch/arm/mach-spear6xx/spear6xx.c
> +++ b/arch/arm/mach-spear6xx/spear6xx.c
> @@ -389,27 +389,27 @@ static struct map_desc spear6xx_io_desc[] __initdata = {
>  	{
>  		.virtual	= VA_SPEAR6XX_ICM1_UART0_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR6XX_ICM1_UART0_BASE),
> -		.length		= SPEAR6XX_ICM1_UART0_SIZE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	}, {
>  		.virtual	= VA_SPEAR6XX_CPU_VIC_PRI_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR6XX_CPU_VIC_PRI_BASE),
> -		.length		= SPEAR6XX_CPU_VIC_PRI_SIZE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	}, {
>  		.virtual	= VA_SPEAR6XX_CPU_VIC_SEC_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR6XX_CPU_VIC_SEC_BASE),
> -		.length		= SPEAR6XX_CPU_VIC_SEC_SIZE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	}, {
>  		.virtual	= VA_SPEAR6XX_ICM3_SYS_CTRL_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR6XX_ICM3_SYS_CTRL_BASE),
> -		.length		= SPEAR6XX_ICM3_MISC_REG_BASE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	}, {
>  		.virtual	= VA_SPEAR6XX_ICM3_MISC_REG_BASE,
>  		.pfn		= __phys_to_pfn(SPEAR6XX_ICM3_MISC_REG_BASE),
> -		.length		= SPEAR6XX_ICM3_MISC_REG_SIZE,
> +		.length		= SZ_4K,
>  		.type		= MT_DEVICE
>  	},
>  };
> -- 
> 1.7.3.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH V5 29/63] ST SPEAr: Changing resource size of amba devices to SZ_4K
From: Russell King - ARM Linux @ 2011-02-19 17:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ec487864395c6c1b6fcd74ab5280d29a7400b197.1295499395.git.viresh.kumar@st.com>

This is another patch which should be reorganized - move the CLCD bits
into the CLCD patch and arrange for it to be applied early on in the
series.

On Thu, Jan 20, 2011 at 12:56:07PM +0530, Viresh Kumar wrote:
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> Signed-off-by: Rajeev Kumar <rajeev-dlh.kumar@st.com>
> ---
>  arch/arm/mach-spear3xx/spear300.c |    2 +-
>  arch/arm/mach-spear3xx/spear320.c |    2 +-
>  arch/arm/mach-spear3xx/spear3xx.c |    2 +-
>  arch/arm/mach-spear6xx/spear6xx.c |    8 +++-----
>  4 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-spear3xx/spear300.c b/arch/arm/mach-spear3xx/spear300.c
> index fceb71f..6989407 100644
> --- a/arch/arm/mach-spear3xx/spear300.c
> +++ b/arch/arm/mach-spear3xx/spear300.c
> @@ -382,7 +382,7 @@ struct amba_device clcd_device = {
>  	},
>  	.res = {
>  		.start = SPEAR300_CLCD_BASE,
> -		.end = SPEAR300_CLCD_BASE + SPEAR300_CLCD_SIZE - 1,
> +		.end = SPEAR300_CLCD_BASE + SZ_4K - 1,
>  		.flags = IORESOURCE_MEM,
>  	},
>  	.dma_mask = ~0,
> diff --git a/arch/arm/mach-spear3xx/spear320.c b/arch/arm/mach-spear3xx/spear320.c
> index 6ab35db..ab4ad8e 100644
> --- a/arch/arm/mach-spear3xx/spear320.c
> +++ b/arch/arm/mach-spear3xx/spear320.c
> @@ -399,7 +399,7 @@ struct amba_device clcd_device = {
>  	},
>  	.res = {
>  		.start = SPEAR320_CLCD_BASE,
> -		.end = SPEAR320_CLCD_BASE + SPEAR320_CLCD_SIZE - 1,
> +		.end = SPEAR320_CLCD_BASE + SZ_4K - 1,
>  		.flags = IORESOURCE_MEM,
>  	},
>  	.dma_mask = ~0,
> diff --git a/arch/arm/mach-spear3xx/spear3xx.c b/arch/arm/mach-spear3xx/spear3xx.c
> index 9d6d125..6be49b3 100644
> --- a/arch/arm/mach-spear3xx/spear3xx.c
> +++ b/arch/arm/mach-spear3xx/spear3xx.c
> @@ -79,7 +79,7 @@ struct amba_device uart_device = {
>  	},
>  	.res = {
>  		.start = SPEAR3XX_ICM1_UART_BASE,
> -		.end = SPEAR3XX_ICM1_UART_BASE + SPEAR3XX_ICM1_UART_SIZE - 1,
> +		.end = SPEAR3XX_ICM1_UART_BASE + SZ_4K - 1,
>  		.flags = IORESOURCE_MEM,
>  	},
>  	.irq = {IRQ_UART, NO_IRQ},
> diff --git a/arch/arm/mach-spear6xx/spear6xx.c b/arch/arm/mach-spear6xx/spear6xx.c
> index f39538e..a948dd4 100644
> --- a/arch/arm/mach-spear6xx/spear6xx.c
> +++ b/arch/arm/mach-spear6xx/spear6xx.c
> @@ -35,7 +35,7 @@ struct amba_device clcd_device = {
>  	},
>  	.res = {
>  		.start = SPEAR6XX_ICM3_CLCD_BASE,
> -		.end = SPEAR6XX_ICM3_CLCD_BASE + SPEAR6XX_ICM3_CLCD_SIZE - 1,
> +		.end = SPEAR6XX_ICM3_CLCD_BASE + SZ_4K - 1,
>  		.flags = IORESOURCE_MEM,
>  	},
>  	.dma_mask = ~0,
> @@ -117,8 +117,7 @@ struct amba_device uart_device[] = {
>  		},
>  		.res = {
>  			.start = SPEAR6XX_ICM1_UART0_BASE,
> -			.end = SPEAR6XX_ICM1_UART0_BASE +
> -				SPEAR6XX_ICM1_UART0_SIZE - 1,
> +			.end = SPEAR6XX_ICM1_UART0_BASE + SZ_4K - 1,
>  			.flags = IORESOURCE_MEM,
>  		},
>  		.irq = {IRQ_UART_0, NO_IRQ},
> @@ -128,8 +127,7 @@ struct amba_device uart_device[] = {
>  		},
>  		.res = {
>  			.start = SPEAR6XX_ICM1_UART1_BASE,
> -			.end = SPEAR6XX_ICM1_UART1_BASE +
> -				SPEAR6XX_ICM1_UART1_SIZE - 1,
> +			.end = SPEAR6XX_ICM1_UART1_BASE + SZ_4K - 1,
>  			.flags = IORESOURCE_MEM,
>  		},
>  		.irq = {IRQ_UART_1, NO_IRQ},
> -- 
> 1.7.3.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH V5 00/63] Updating SPEAr Support
From: viresh kumar @ 2011-02-19 17:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110219134743.GE29493@n2100.arm.linux.org.uk>

On 2/19/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> I don't understand why people think that it's a good idea to post a
> patch series which adds new support and then has a whole series of
> patches which modify then modifies both new and old stuff.
>
> It makes it a lot harder to apply, a lot harder to review, and all
> round is a bad idea.
>
> The problem is that mixing it up in the way you've done means that
> it's impossible to apply the cleanup patches before the new support.
>

Yes, understood it well now. This will not happen again.

> Another issue worth pointing out is that submission of patch series
> by email has to be done _carefully_ - which basically means slowly.
> The email system on this planet will randomly re-order messages -
> there is no guarantee that the order in which you send them is the
> order in which they're received at the remote end.  If you want them
> to arrive in a certain order, you're out of luck.
>
> You can help by not dumping 63 patches into the mail system within a
> few seconds or so, but leaving a greater period of time between each.
> The longer that period, the more likely they are to arrive in order -
> but it's something that will never be guaranteed.
>

As that is never guaranteed, can we have some other way to work out this issue?
I hope the issue is regarding the correct ordering of patches, as you
don't see the
numbering of these in the patch tracker? Is there anything more to it?

If this is the case, then may be we can send cover-letter along with
patch series
mentioning correct order of patches in it, and keeping you in cc of
cover-letter alone, so that
cover-letter is not lost by tracker, as it only takes patches.

> That's another argument for not waiting until you have a huge patch
> set before sending stuff out - keep patch sets small and targetted to
> specific areas.  For instance, with the Versatile updates which form a
> total of 21 individual patches, I split them up into one set of 10
> patches which changed the way CLCD stuff was handled, and another
> separate set of 11 patches of platform specific stuff.
>

Ok.

> So, why aren't all the padmux changes together?  Why aren't the clk
> API patches together?
>

We didn't thought you will be required to do rework on this. And
thought you will apply
this patch series as it is. But now as we see, you need to put extra
efforts due to our
mistakes, we will not repeat them.

> And another thing: separate out the 'fixes' from the new developments.
> Things like the addition of UL to VMALLOC_END counts as a warning fix.
> Using readl_relaxed/writel_relaxed also counts as a fix as things in
> the decompressor break.  As these are technically bug fixes, they go
> upstream faster than new work.
>
> Bug fixes should _always_ be at the very start of any patch series,
> then cleanups, then new stuff.
>

Ok.

> In summary, I hate large patch sets.  I tend to ignore large patch
> sets.  I have very little motivation to review large patch sets.  I
> have very little motivation to apply large patch sets.  Don't create
> them in the first place.  Spend time sorting the patches into smaller
> sets and things all round go much easier.
>
> I'll work through what you've already submitted, but it will be a
> slow and erratic job.
>

Sorry for that again!! And thanks for your effort and help.

--
viresh

^ permalink raw reply

* [PATCH V5 57/63] ST SPEAr3xx: Updating plgpio and emi source to make it compliant with single image strategy
From: viresh kumar @ 2011-02-19 17:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110219165042.GN29493@n2100.arm.linux.org.uk>

On 2/19/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Thu, Jan 20, 2011 at 12:56:35PM +0530, Viresh Kumar wrote:
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>
> Why aren't the plgpio bits of this in patch 16?
>

Because we made these changes after that single image solution. And
yes we should
have done our homework on this cleanly and merged all this in patch 16
and emi's patch.

>> +static u32 plgpio_enb, plgpio_wdata, plgpio_dir, plgpio_rdata, plgpio_ie,
>> +	   plgpio_mis;
>
> 'int' or 'unsigned int' will do for these.  There's no pressing need for
> these to be exactly 32-bit quantities.
>

Yes.

>>  static int __init plgpio_init(void)
>>  {
>> +	if (machine_is_spear310()) {
>> +		plgpio_enb = SPEAR310_PLGPIO_ENB;
>> +		plgpio_wdata = SPEAR310_PLGPIO_WDATA;
>> +		plgpio_dir = SPEAR310_PLGPIO_DIR;
>> +		plgpio_rdata = SPEAR310_PLGPIO_IE;
>> +		plgpio_ie = SPEAR310_PLGPIO_RDATA;
>> +		plgpio_mis = SPEAR310_PLGPIO_MIS;
>> +	} else if (machine_is_spear320()) {
>> +		plgpio_enb = SPEAR320_PLGPIO_ENB;
>> +		plgpio_wdata = SPEAR320_PLGPIO_WDATA;
>> +		plgpio_dir = SPEAR320_PLGPIO_DIR;
>> +		plgpio_rdata = SPEAR320_PLGPIO_IE;
>> +		plgpio_ie = SPEAR320_PLGPIO_RDATA;
>> +		plgpio_mis = SPEAR320_PLGPIO_MIS;
>> +	} else {
>> +		return 0;
>
> If it's not supported, then what about returning an error instead of
> zero?
>

Yes. Error must be returned.

> Can you supersede this patch and patch 16 in the patch system with a
> single patch which adds PLGPIO support with no further updates.
>
> You may also consider combining the EMI updates with the patch which
> creates EMI support in the first place, which I think is patch 36.
>

Will do this for both plgpio and emi.

^ permalink raw reply

* [PATCH V5 15/63] ST SPEAr: Correcting SOC Config base address for spear320
From: viresh kumar @ 2011-02-19 17:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110219164020.GM29493@n2100.arm.linux.org.uk>

On 2/19/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Thu, Jan 20, 2011 at 12:55:54PM +0530, Viresh Kumar wrote:
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>
> This looks to me like another bug fix, and I'm treating it as such.  It
> should be submitted separately from this patch set along with other bug
> fixes, and clearly labelled as such.
>

Yes it is a bug fix and should have been submitted separately.

> I've also changed the comments on this patch to:
>
> "SPEAr: Correct SOC config base address for spear320
>
> SPEAR320_SOC_CONFIG_BASE was wrong, causing the wrong registers to be
> accessed."
>

Thanks for your help. Will take care of commit message next time.

--
viresh

^ permalink raw reply

* [PATCH V5 41/63] ST SPEAr: replace readl, writel with readl_relaxed, writel_relaxed in uncompress.h
From: viresh kumar @ 2011-02-19 17:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110219144439.GG29493@n2100.arm.linux.org.uk>

On 2/19/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Thu, Jan 20, 2011 at 12:56:19PM +0530, Viresh Kumar wrote:
>> readl also calls outer cache maintainance operations which are not
>> available
>> during Linux uncompression. This patch replaces readl, writel with
>> readl_relaxed
>> and writel_relaxed.
>
> Shouldn't this be near the start of the series?
>

Yes.

> Also, would:
>
> "SPEAr: replace readl, writel with relaxed versions in uncompress.h
>
> readl() and writel() calls the outer cache maintainance operations
> which are not available during Linux uncompression. This patch replaces
> readl() and writel() with readl_relaxed() and writel_relaxed() to avoid
> the link time errors."
>
> be a better patch description?
>

Obviously. Will take care of such things next time we send a patch.

--
viresh

^ permalink raw reply

* [PATCH V5 13/63] ST SPEAr: Updating Clock Support
From: Russell King - ARM Linux @ 2011-02-19 17:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b136f1242af026a0453de2c56b88bce55b41e222.1295499394.git.viresh.kumar@st.com>

On Thu, Jan 20, 2011 at 12:55:52PM +0530, Viresh Kumar wrote:
>  arch/arm/mach-spear13xx/clock.c                  |  621 +++++++++++++++++++-
>  arch/arm/mach-spear13xx/include/mach/misc_regs.h |  101 ++--

As I said in the patch system, I think this should be a preparatory
patch before adding spear13xx support.  So I've committed this patch
without the spear13xx changes.  When I commit the spear13xx changes
I'll fold them into that patch.

^ permalink raw reply

* [PATCH V5 03/63] sp810 Fix: Switch to slow mode before sysctl_soft_reset
From: viresh kumar @ 2011-02-19 17:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110219144044.GF29493@n2100.arm.linux.org.uk>

On 2/19/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Thu, Jan 20, 2011 at 12:55:42PM +0530, Viresh Kumar wrote:
>> From: Shiraz Hashim <shiraz.hashim@st.com>
>>
>> Before resetting system from system controller, switching to slow
>> mode is required.
>
> This comment could do with some expansion about why it is required.
> Eg, this comment could become:
>
> "sp810: switch to slow mode before reset
>
> In sysctl_soft_reset(), switch to slow mode before resetting the system
> via the system controller.  This is required by the SPEAr reference
> manual."
>

Yes. Will take care of such things next time.

--
viresh

^ permalink raw reply

* [PATCH V5 36/63] ST SPEAr : EMI (Extrenal Memory Interface) controller driver
From: Russell King - ARM Linux @ 2011-02-19 17:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <942264b07d606a02eb68561c7be559b6a4904961.1295499395.git.viresh.kumar@st.com>

On Thu, Jan 20, 2011 at 12:56:14PM +0530, Viresh Kumar wrote:
> From: Vipin Kumar <vipin.kumar@st.com>


Subject: [PATCH V5 36/63] ST SPEAr : EMI (Extrenal Memory Interface)
        controller driver

"External"

> 2 SPEAr platform SoCs(spear310 and spear320) support an External Memory
> Interface controller. This controller is used to interface with
> Parallel NOR Flash devices.
> 
> This patch adds just the platform code needed for EMI (mainly EMI
> initialization). The driver being used is driver/mtd/maps/physmap.c
> 
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  arch/arm/mach-spear3xx/Makefile                |    4 +
>  arch/arm/mach-spear3xx/emi.c                   |  101 ++++++++++++++++++++++++
>  arch/arm/mach-spear3xx/include/mach/emi.h      |   64 +++++++++++++++
>  arch/arm/mach-spear3xx/include/mach/generic.h  |    2 +
>  arch/arm/mach-spear3xx/include/mach/spear310.h |    9 ++
>  arch/arm/mach-spear3xx/include/mach/spear320.h |    6 ++
>  arch/arm/mach-spear3xx/spear310.c              |    9 ++
>  arch/arm/mach-spear3xx/spear310_evb.c          |   28 +++++++
>  arch/arm/mach-spear3xx/spear320.c              |    9 ++
>  arch/arm/mach-spear3xx/spear320_evb.c          |   27 ++++++
>  10 files changed, 259 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-spear3xx/emi.c
>  create mode 100644 arch/arm/mach-spear3xx/include/mach/emi.h
> 
> diff --git a/arch/arm/mach-spear3xx/Makefile b/arch/arm/mach-spear3xx/Makefile
> index b248624..d38ae47 100644
> --- a/arch/arm/mach-spear3xx/Makefile
> +++ b/arch/arm/mach-spear3xx/Makefile
> @@ -24,3 +24,7 @@ obj-$(CONFIG_MACH_SPEAR320) += spear320.o
>  
>  # spear320 boards files
>  obj-$(CONFIG_BOARD_SPEAR320_EVB) += spear320_evb.o
> +
> +# specific files
> +obj-$(CONFIG_MACH_SPEAR310) += emi.o
> +obj-$(CONFIG_MACH_SPEAR320) += emi.o
> diff --git a/arch/arm/mach-spear3xx/emi.c b/arch/arm/mach-spear3xx/emi.c
> new file mode 100644
> index 0000000..7b62ff0
> --- /dev/null
> +++ b/arch/arm/mach-spear3xx/emi.c
> @@ -0,0 +1,101 @@
> +/*
> + * arch/arm/mach-spear3xx/emi.c
> + *
> + * EMI (External Memory Interface) file
> + *
> + * Copyright (C) 2010 ST Microelectronics
> + * Vipin Kumar<vipin.kumar@st.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <mach/emi.h>
> +
> +int __init emi_init(struct platform_device *pdev, unsigned long base,
> +		u32 bank, u32 width)
> +{
> +	void __iomem *emi_reg_base;
> +	struct clk *clk;
> +	int ret;
> +
> +	if (bank > (EMI_MAX_BANKS - 1))
> +		return -EINVAL;
> +
> +	emi_reg_base = ioremap(base, EMI_REG_SIZE);
> +	if (!emi_reg_base)
> +		return -ENOMEM;
> +
> +	clk = clk_get(NULL, "emi");

What is the platform device pointer passed into this function?  It seems
unused - would using &pdev->dev here be appropriate?  Possibly not as it
seems to be the physmap device.

> +	if (IS_ERR(clk)) {
> +		iounmap(emi_reg_base);
> +		return PTR_ERR(clk);
> +	}
> +
> +	ret = clk_enable(clk);
> +	if (ret) {
> +		iounmap(emi_reg_base);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Note: These are relaxed NOR device timings. Nor devices on spear
> +	 * eval machines are working fine with these timings. Specific board
> +	 * files can optimize these timings based on devices found on board.
> +	 */
> +	writel(0x10, emi_reg_base + (EMI_BANK_REG_SIZE * bank) + TAP_REG);
> +	writel(0x05, emi_reg_base + (EMI_BANK_REG_SIZE * bank) + TSDP_REG);
> +	writel(0x0a, emi_reg_base + (EMI_BANK_REG_SIZE * bank) + TDPW_REG);
> +	writel(0x0a, emi_reg_base + (EMI_BANK_REG_SIZE * bank) + TDPR_REG);
> +	writel(0x05, emi_reg_base + (EMI_BANK_REG_SIZE * bank) + TDCS_REG);
> +
> +	switch (width) {
> +	case EMI_FLASH_WIDTH8:
> +		width = EMI_CNTL_WIDTH8;
> +		break;
> +
> +	case EMI_FLASH_WIDTH16:
> +		width = EMI_CNTL_WIDTH16;
> +		break;
> +
> +	case EMI_FLASH_WIDTH32:
> +		width = EMI_CNTL_WIDTH32;
> +		break;
> +	default:
> +		width = EMI_CNTL_WIDTH8;
> +		break;
> +	}
> +	/* set the data width */
> +	writel(width | EMI_CNTL_ENBBYTERW,
> +		emi_reg_base + (EMI_BANK_REG_SIZE * bank) + CTRL_REG);
> +
> +	/* disable all the acks */
> +	writel(0x3f, emi_reg_base + ack_reg);
> +
> +	iounmap(emi_reg_base);
> +
> +	return 0;
> +}
> +
> +void __init
> +emi_init_board_info(struct platform_device *pdev, struct resource *resources,
> +		int res_num, struct mtd_partition *partitions,
> +		unsigned int nr_partitions, unsigned int width)
> +{
> +	struct physmap_flash_data *emi_plat_data = dev_get_platdata(&pdev->dev);
> +
> +	pdev->resource = resources;
> +	pdev->num_resources = res_num;
> +
> +	if (partitions) {
> +		emi_plat_data->parts = partitions;
> +		emi_plat_data->nr_parts = nr_partitions;
> +	}
> +
> +	emi_plat_data->width = width;
> +}

I don't see why this has to be code rather than in the platform specific
files as static initializers.  Also, you seem to be passing stuff like
'EMI_FLASH_WIDTH32' in for 'width', which seems very odd as this is the
byte width.  I don't see why you need #defined constants for this.

It's waiting someone who doesn't realise that it has a proper meaning
to change it to an enum or something like that.

If you think the physmap flash API is lacking some definitions for this,
please talk to the MTD people.

^ permalink raw reply

* [PATCH V5 57/63] ST SPEAr3xx: Updating plgpio and emi source to make it compliant with single image strategy
From: Russell King - ARM Linux @ 2011-02-19 16:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <29f3e319e062a9f2f65ea9683fb9663f938dfa3b.1295499395.git.viresh.kumar@st.com>

On Thu, Jan 20, 2011 at 12:56:35PM +0530, Viresh Kumar wrote:
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>

Why aren't the plgpio bits of this in patch 16?

> +static u32 plgpio_enb, plgpio_wdata, plgpio_dir, plgpio_rdata, plgpio_ie,
> +	   plgpio_mis;

'int' or 'unsigned int' will do for these.  There's no pressing need for
these to be exactly 32-bit quantities.

>  static int __init plgpio_init(void)
>  {
> +	if (machine_is_spear310()) {
> +		plgpio_enb = SPEAR310_PLGPIO_ENB;
> +		plgpio_wdata = SPEAR310_PLGPIO_WDATA;
> +		plgpio_dir = SPEAR310_PLGPIO_DIR;
> +		plgpio_rdata = SPEAR310_PLGPIO_IE;
> +		plgpio_ie = SPEAR310_PLGPIO_RDATA;
> +		plgpio_mis = SPEAR310_PLGPIO_MIS;
> +	} else if (machine_is_spear320()) {
> +		plgpio_enb = SPEAR320_PLGPIO_ENB;
> +		plgpio_wdata = SPEAR320_PLGPIO_WDATA;
> +		plgpio_dir = SPEAR320_PLGPIO_DIR;
> +		plgpio_rdata = SPEAR320_PLGPIO_IE;
> +		plgpio_ie = SPEAR320_PLGPIO_RDATA;
> +		plgpio_mis = SPEAR320_PLGPIO_MIS;
> +	} else {
> +		return 0;

If it's not supported, then what about returning an error instead of
zero?

Can you supersede this patch and patch 16 in the patch system with a
single patch which adds PLGPIO support with no further updates.

You may also consider combining the EMI updates with the patch which
creates EMI support in the first place, which I think is patch 36.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox