Skip to contentSkip to conversation

clk: mmp: use global clock lock

PATCH RESEND New Daniel Drake 2 participants 3 messages Jul 16, 2013 – Aug 8, 2013 Common clk

lore
Share & tools
lore

Daniel Drake

author
#0 lore raw

PATCH RESEND 1/2 clk: mmp: use global clock lock

The mmp platform shares a lock between different clock types, and all clocks are currently initialized by static bindings.

Move this static/private lock into global context so that it can easily be shared by clocks that will be defined by the device tree in followup patches.

Signed-off-by Daniel Drake <[email protected]>
---
 drivers/clk/mmp/Makefile   |   2 +-
 drivers/clk/mmp/clk-apbc.c |  38 +++++----------
 drivers/clk/mmp/clk-apmu.c |  20 ++------
 drivers/clk/mmp/clk-mmp2.c | 116 ++++++++++++++++++++++-----------------------
 drivers/clk/mmp/clk.h      |   8 ++--
 drivers/clk/mmp/common.c   |  13 +++++
 6 files changed, 92 insertions(+), 105 deletions(-)
 create mode 100644 drivers/clk/mmp/common.c

Resending after a month without review.

drivers/clk/mmp/Makefile +1 -1
index 392d780..31aacb5 100644
@@ -2,7 +2,7 @@
 # Makefile for mmp specific clk
 #
 
-obj-y += clk-apbc.o clk-apmu.o clk-frac.o
+obj-y += common.o clk-apbc.o clk-apmu.o clk-frac.o
 
 obj-$(CONFIG_CPU_PXA168) += clk-pxa168.o
 obj-$(CONFIG_CPU_PXA910) += clk-pxa910.o
drivers/clk/mmp/clk-apbc.c +11 -27
index d14120e..89a146a 100644
@@ -30,7 +30,6 @@ struct clk_apbc {
 	void __iomem		*base;
 	unsigned int		delay;
 	unsigned int		flags;
-	spinlock_t		*lock;
 };
 
 static int clk_apbc_prepare(struct clk_hw *hw)

@@ -43,8 +42,7 @@ static int clk_apbc_prepare(struct clk_hw *hw)
 	 * It may share same register as MUX clock,
 	 * and it will impact FNCLK enable. Spinlock is needed
 	 */
-	if (apbc->lock)
-		spin_lock_irqsave(apbc->lock, flags);
+	spin_lock_irqsave(&mmp_clk_lock, flags);
 
 	data = readl_relaxed(apbc->base);
 	if (apbc->flags & APBC_POWER_CTRL)

@@ -52,33 +50,26 @@ static int clk_apbc_prepare(struct clk_hw *hw)
 	data |= APBC_FNCLK;
 	writel_relaxed(data, apbc->base);
 
-	if (apbc->lock)
-		spin_unlock_irqrestore(apbc->lock, flags);
-
+	spin_unlock_irqrestore(&mmp_clk_lock, flags);
 	udelay(apbc->delay);
-
-	if (apbc->lock)
-		spin_lock_irqsave(apbc->lock, flags);
+	spin_lock_irqsave(&mmp_clk_lock, flags);
 
 	data = readl_relaxed(apbc->base);
 	data |= APBC_APBCLK;
 	writel_relaxed(data, apbc->base);
 
-	if (apbc->lock)
-		spin_unlock_irqrestore(apbc->lock, flags);
+	spin_unlock_irqrestore(&mmp_clk_lock, flags);
 
 	udelay(apbc->delay);
 
 	if (!(apbc->flags & APBC_NO_BUS_CTRL)) {
-		if (apbc->lock)
-			spin_lock_irqsave(apbc->lock, flags);
+		spin_lock_irqsave(&mmp_clk_lock, flags);
 
 		data = readl_relaxed(apbc->base);
 		data &= ~APBC_RST;
 		writel_relaxed(data, apbc->base);
 
-		if (apbc->lock)
-			spin_unlock_irqrestore(apbc->lock, flags);
+		spin_unlock_irqrestore(&mmp_clk_lock, flags);
 	}
 
 	return 0;

@@ -90,8 +81,7 @@ static void clk_apbc_unprepare(struct clk_hw *hw)
 	unsigned long data;
 	unsigned long flags = 0;
 
-	if (apbc->lock)
-		spin_lock_irqsave(apbc->lock, flags);
+	spin_lock_irqsave(&mmp_clk_lock, flags);
 
 	data = readl_relaxed(apbc->base);
 	if (apbc->flags & APBC_POWER_CTRL)

@@ -99,20 +89,15 @@ static void clk_apbc_unprepare(struct clk_hw *hw)
 	data &= ~APBC_FNCLK;
 	writel_relaxed(data, apbc->base);
 
-	if (apbc->lock)
-		spin_unlock_irqrestore(apbc->lock, flags);
-
+	spin_unlock_irqrestore(&mmp_clk_lock, flags);
 	udelay(10);
-
-	if (apbc->lock)
-		spin_lock_irqsave(apbc->lock, flags);
+	spin_lock_irqsave(&mmp_clk_lock, flags);
 
 	data = readl_relaxed(apbc->base);
 	data &= ~APBC_APBCLK;
 	writel_relaxed(data, apbc->base);
 
-	if (apbc->lock)
-		spin_unlock_irqrestore(apbc->lock, flags);
+	spin_unlock_irqrestore(&mmp_clk_lock, flags);
 }
 
 struct clk_ops clk_apbc_ops = {

@@ -122,7 +107,7 @@ struct clk_ops clk_apbc_ops = {
 
 struct clk *mmp_clk_register_apbc(const char *name, const char *parent_name,
 		void __iomem *base, unsigned int delay,
-		unsigned int apbc_flags, spinlock_t *lock)
+		unsigned int apbc_flags)
 {
 	struct clk_apbc *apbc;
 	struct clk *clk;

@@ -141,7 +126,6 @@ struct clk *mmp_clk_register_apbc(const char *name, const char *parent_name,
 	apbc->base = base;
 	apbc->delay = delay;
 	apbc->flags = apbc_flags;
-	apbc->lock = lock;
 	apbc->hw.init = &init;
 
 	clk = clk_register(NULL, &apbc->hw);
drivers/clk/mmp/clk-apmu.c +5 -15
index abe182b..4f4d8c5 100644
@@ -24,7 +24,6 @@ struct clk_apmu {
 	void __iomem    *base;
 	u32		rst_mask;
 	u32		enable_mask;
-	spinlock_t	*lock;
 };
 
 static int clk_apmu_enable(struct clk_hw *hw)

@@ -33,14 +32,10 @@ static int clk_apmu_enable(struct clk_hw *hw)
 	unsigned long data;
 	unsigned long flags = 0;
 
-	if (apmu->lock)
-		spin_lock_irqsave(apmu->lock, flags);
-
+	spin_lock_irqsave(&mmp_clk_lock, flags);
 	data = readl_relaxed(apmu->base) | apmu->enable_mask;
 	writel_relaxed(data, apmu->base);
-
-	if (apmu->lock)
-		spin_unlock_irqrestore(apmu->lock, flags);
+	spin_unlock_irqrestore(&mmp_clk_lock, flags);
 
 	return 0;
 }

@@ -51,14 +46,10 @@ static void clk_apmu_disable(struct clk_hw *hw)
 	unsigned long data;
 	unsigned long flags = 0;
 
-	if (apmu->lock)
-		spin_lock_irqsave(apmu->lock, flags);
-
+	spin_lock_irqsave(&mmp_clk_lock, flags);
 	data = readl_relaxed(apmu->base) & ~apmu->enable_mask;
 	writel_relaxed(data, apmu->base);
-
-	if (apmu->lock)
-		spin_unlock_irqrestore(apmu->lock, flags);
+	spin_unlock_irqrestore(&mmp_clk_lock, flags);
 }
 
 struct clk_ops clk_apmu_ops = {

@@ -67,7 +58,7 @@ struct clk_ops clk_apmu_ops = {
 };
 
 struct clk *mmp_clk_register_apmu(const char *name, const char *parent_name,
-		void __iomem *base, u32 enable_mask, spinlock_t *lock)
+		void __iomem *base, u32 enable_mask)
 {
 	struct clk_apmu *apmu;
 	struct clk *clk;

@@ -85,7 +76,6 @@ struct clk *mmp_clk_register_apmu(const char *name, const char *parent_name,
 
 	apmu->base = base;
 	apmu->enable_mask = enable_mask;
-	apmu->lock = lock;
 	apmu->hw.init = &init;
 
 	clk = clk_register(NULL, &apmu->hw);
drivers/clk/mmp/clk-mmp2.c +57 -59

319 lines not on this page · raw message

drivers/clk/mmp/clk.h +5 -3
index ab86dd4..d697d9b 100644
@@ -3,6 +3,9 @@
 
 #include <linux/clk-provider.h>
 #include <linux/clkdev.h>
+#include <linux/spinlock.h>
+
+extern spinlock_t mmp_clk_lock;
 
 #define APBC_NO_BUS_CTRL	BIT(0)
 #define APBC_POWER_CTRL		BIT(1)

@@ -24,10 +27,9 @@ extern struct clk *mmp_clk_register_pll2(const char *name,
 		const char *parent_name, unsigned long flags);
 extern struct clk *mmp_clk_register_apbc(const char *name,
 		const char *parent_name, void __iomem *base,
-		unsigned int delay, unsigned int apbc_flags, spinlock_t *lock);
+		unsigned int delay, unsigned int apbc_flags);
 extern struct clk *mmp_clk_register_apmu(const char *name,
-		const char *parent_name, void __iomem *base, u32 enable_mask,
-		spinlock_t *lock);
+		const char *parent_name, void __iomem *base, u32 enable_mask);
 extern struct clk *mmp_clk_register_factor(const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *base, struct clk_factor_masks *masks,
drivers/clk/mmp/common.c new file +13 -0
new file mode 100644
index 0000000..dadce5f
@@ -0,0 +1,13 @@
+/*
+ * mmp clock common elements
+ *
+ * Copyright (C) 2013 One Laptop per Child
+ *
+ * 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/spinlock.h>
+
+DEFINE_SPINLOCK(mmp_clk_lock);
Signature · 1 line
1.8.1.4
#1 lore raw

PATCH RESEND 1/2 clk: mmp: use global clock lock

Quoted 18 lines · Daniel Drake

The mmp platform shares a lock between different clock types, and all clocks are currently initialized by static bindings.

Move this static/private lock into global context so that it can easily be shared by clocks that will be defined by the device tree in followup patches.

Signed-off-by Daniel Drake <[email protected]>
---
 drivers/clk/mmp/Makefile   |   2 +-
 drivers/clk/mmp/clk-apbc.c |  38 +++++----------
 drivers/clk/mmp/clk-apmu.c |  20 ++------
 drivers/clk/mmp/clk-mmp2.c | 116 ++++++++++++++++++++++-----------------------
 drivers/clk/mmp/clk.h      |   8 ++--
 drivers/clk/mmp/common.c   |  13 +++++
 6 files changed, 92 insertions(+), 105 deletions(-)
 create mode 100644 drivers/clk/mmp/common.c
Quoted 2 lines

Resending after a month without review.

Currently lock is defined in pxa910, pxa168 & mmp2 clock driver. Only one of these driver
could be active in real system. So I can't see any benefit to use the shared clock lock among
these three drivers.

Regards
Haojian

author
#2 lore raw

PATCH RESEND 1/2 clk: mmp: use global clock lock

Hi,

Sorry for the late response.

Quoted 5 lines · Haojian Zhuang

Currently lock is defined in pxa910, pxa168 & mmp2 clock driver. Only one of these driver
could be active in real system. So I can't see any benefit to use the shared clock lock among
these three drivers.

As far as I can see, my patch does not attempt to share the clock between those 3 platforms. Instead it only affects the mmp2 case.

The mmp2 case does *already* have a shared clock lock defined in static bindings (drivers/clk/mmp/clk-mmp2.c):
static DEFINE_SPINLOCK(clk_lock);

This lock is shared between MMP apmu and apbc drivers.

My patch simply moves the location of where the shared clk_lock is defined so that it can be additionally used by dynamic DT-driven bindings. If you take a quick glance at the next patch, "clk: mmp: add support for DT-defined clocks" the reasoning behind this change is hopefully even clearer.

Thanks
Daniel