Skip to contentSkip to conversation

ARM: sa1100: use clock api to get cpu freq in fb and pcmcia drivers

PATCH New Dmitry Eremin-Solenikov 1 participant 7 messages Nov 24, 2014

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v2 cover ARM: sa1100: use clock api to get cpu freq in fb and pcmcia drivers

[Changes since V1:
- reorder patches to add sa1100 and sa1111 clocks before using them in sa1100 pcmcia driver.
- Fix error returns in case clk_get returns an error.
- Call clk_prepare_enable/clk_disable_unprepare for the proper API usage.]

On SA-1100 framebuffer and PCMCIA drivers make use of cpufreq_get(0) function call to determine the cpu frequency. Russell's commit 1937f5b91833e2e8e53bcc821fc7a5fbe6ccb9b5 (ARM: fix sa1100 build) fixed the build issues, but broke two devices (Collie and Jornada720). For those two boards the cpufreq code gets compiled but is not enabled (as board files do not provide timing information for the CPUFREQ driver). Thus cpufreq_get(0) returns incorrect value and incorrect timings get programmed into the hardware.

PXA2xx (the very similar platform) uses Clock API to determine CPU frequency both in framebuffer and PCMCIA drivers. These patches make similar changes to StrongARM drivers.

--
With best wishes
Dmitry

#1 lore raw

PATCH v2 1/6 ARM: sa1100: add cpu clock

Both SA1100 framebuffer and PCMCIA drivers require knowledge of cpu frequency to correctly program timings. Currently they receive timing information by calling cpufreq_get(0). However if cpu frequency driver is not enabled (e.g. due to unsupported DRAM chip/board on sa1110) cpufreq_get(0) returns 0, causing incorrect timings to be programmed.

Add cpu clock returning cpu frequency, to be used by sa11x0 fb and pcmcia drivers.

---
 arch/arm/mach-sa1100/clock.c | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)
arch/arm/mach-sa1100/clock.c +34 -7
index 9fa6a99..53f750d 100644
@@ -15,10 +15,12 @@
 #include <linux/clkdev.h>
 
 #include <mach/hardware.h>
+#include <mach/generic.h>
 
 struct clkops {
 	void			(*enable)(struct clk *);
 	void			(*disable)(struct clk *);
+	unsigned long		(*get_rate)(struct clk *);
 };
 
 struct clk {
@@ -33,13 +35,6 @@ struct clk clk_##_name = {				\
 
 static DEFINE_SPINLOCK(clocks_lock);
 
-/* Dummy clk routine to build generic kernel parts that may be using them */
-unsigned long clk_get_rate(struct clk *clk)
-{
-	return 0;
-}
-EXPORT_SYMBOL(clk_get_rate);
-
 static void clk_gpio27_enable(struct clk *clk)
 {
 	/*
@@ -58,6 +53,19 @@ static void clk_gpio27_disable(struct clk *clk)
 	GAFR &= ~GPIO_32_768kHz;
 }
 
+static void clk_cpu_enable(struct clk *clk)
+{
+}
+
+static void clk_cpu_disable(struct clk *clk)
+{
+}
+
+static unsigned long clk_cpu_get_rate(struct clk *clk)
+{
+	return sa11x0_getspeed(0) * 1000;
+}
+
 int clk_enable(struct clk *clk)
 {
 	unsigned long flags;
@@ -87,16 +95,35 @@ void clk_disable(struct clk *clk)
 }
 EXPORT_SYMBOL(clk_disable);
 
+unsigned long clk_get_rate(struct clk *clk)
+{
+	if (clk && clk->ops && clk->ops->get_rate)
+		return clk->ops->get_rate(clk);
+
+	return 0;
+}
+EXPORT_SYMBOL(clk_get_rate);
+
 const struct clkops clk_gpio27_ops = {
 	.enable		= clk_gpio27_enable,
 	.disable	= clk_gpio27_disable,
 };
 
+const struct clkops clk_cpu_ops = {
+	.enable		= clk_cpu_enable,
+	.disable	= clk_cpu_disable,
+	.get_rate	= clk_cpu_get_rate,
+};
+
 static DEFINE_CLK(gpio27, &clk_gpio27_ops);
 
+static DEFINE_CLK(cpu, &clk_cpu_ops);
+
 static struct clk_lookup sa11xx_clkregs[] = {
 	CLKDEV_INIT("sa1111.0", NULL, &clk_gpio27),
 	CLKDEV_INIT("sa1100-rtc", NULL, NULL),
+	CLKDEV_INIT("sa11x0-fb", NULL, &clk_cpu),
+	CLKDEV_INIT("sa11x0-pcmcia", NULL, &clk_cpu),
 };
 
 static int __init sa11xx_clk_init(void)
Signature · 1 line
2.1.3
#2 lore raw

PATCH v2 2/6 ARM: sa1100: add a clock alias for sa1111 pcmcia device

In reply to Dmitry Eremin-Solenikov

SA-1111 uses internal MMIO space offsets as a device name, so device name for sa1111 pcmcia is 1800 (PCMCIA is at offset 0x1800).

---
 arch/arm/mach-sa1100/clock.c | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/mach-sa1100/clock.c +2 -0
index 53f750d..03c75a8 100644
@@ -124,6 +124,8 @@ static struct clk_lookup sa11xx_clkregs[] = {
 	CLKDEV_INIT("sa1100-rtc", NULL, NULL),
 	CLKDEV_INIT("sa11x0-fb", NULL, &clk_cpu),
 	CLKDEV_INIT("sa11x0-pcmcia", NULL, &clk_cpu),
+	/* sa1111 names devices using internal offsets, PCMCIA is at 0x1800 */
+	CLKDEV_INIT("1800", NULL, &clk_cpu),
 };
 
 static int __init sa11xx_clk_init(void)
Signature · 1 line
2.1.3
#3 lore raw

PATCH v2 3/6 fbdev: sa1100fb: make use of device clock

In reply to Dmitry Eremin-Solenikov

Use per-device clock (instead of calling cpufreq_get(0), which can return 0 if no cpu frequency driver is selected) to program timings.

---
 drivers/video/fbdev/sa1100fb.c | 30 +++++++++++++++++++++++-------
 drivers/video/fbdev/sa1100fb.h |  1 +
 2 files changed, 24 insertions(+), 7 deletions(-)
drivers/video/fbdev/sa1100fb.c +23 -7
index 580c444e..2133cf1 100644
@@ -178,6 +178,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/mutex.h>
 #include <linux/io.h>
+#include <linux/clk.h>
 
 #include <video/sa1100fb.h>
 
@@ -413,9 +414,9 @@ sa1100fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 		var->transp.offset);
 
 #ifdef CONFIG_CPU_FREQ
-	dev_dbg(fbi->dev, "dma period = %d ps, clock = %d kHz\n",
+	dev_dbg(fbi->dev, "dma period = %d ps, clock = %ld kHz\n",
 		sa1100fb_display_dma_period(var),
-		cpufreq_get(smp_processor_id()));
+		clk_get_rate(fbi->clk) / 1000);
 #endif
 
 	return 0;
@@ -586,9 +587,10 @@ static struct fb_ops sa1100fb_ops = {
  * Calculate the PCD value from the clock rate (in picoseconds).
  * We take account of the PPCR clock setting.
  */
-static inline unsigned int get_pcd(unsigned int pixclock, unsigned int cpuclock)
+static inline unsigned int get_pcd(struct sa1100fb_info *fbi,
+		unsigned int pixclock)
 {
-	unsigned int pcd = cpuclock / 100;
+	unsigned int pcd = clk_get_rate(fbi->clk) / 100 / 1000;
 
 	pcd *= pixclock;
 	pcd /= 10000000;
@@ -667,7 +669,7 @@ static int sa1100fb_activate_var(struct fb_var_screeninfo *var, struct sa1100fb_
 		LCCR2_BegFrmDel(var->upper_margin) +
 		LCCR2_EndFrmDel(var->lower_margin);
 
-	pcd = get_pcd(var->pixclock, cpufreq_get(0));
+	pcd = get_pcd(fbi, var->pixclock);
 	new_regs.lccr3 = LCCR3_PixClkDiv(pcd) | fbi->inf->lccr3 |
 		(var->sync & FB_SYNC_HOR_HIGH_ACT ? LCCR3_HorSnchH : LCCR3_HorSnchL) |
 		(var->sync & FB_SYNC_VERT_HIGH_ACT ? LCCR3_VrtSnchH : LCCR3_VrtSnchL);
@@ -781,6 +783,9 @@ static void sa1100fb_enable_controller(struct sa1100fb_info *fbi)
 	fbi->palette_cpu[0] &= 0xcfff;
 	fbi->palette_cpu[0] |= palette_pbs(&fbi->fb.var);
 
+	/* enable LCD controller clock */
+	clk_prepare_enable(fbi->clk);
+
 	/* Sequence from 11.7.10 */
 	writel_relaxed(fbi->reg_lccr3, fbi->base + LCCR3);
 	writel_relaxed(fbi->reg_lccr2, fbi->base + LCCR2);
@@ -825,6 +830,9 @@ static void sa1100fb_disable_controller(struct sa1100fb_info *fbi)
 
 	schedule_timeout(20 * HZ / 1000);
 	remove_wait_queue(&fbi->ctrlr_wait, &wait);
+
+	/* disable LCD controller clock */
+	clk_disable_unprepare(fbi->clk);
 }
 
 /*
@@ -1003,7 +1011,6 @@ sa1100fb_freq_transition(struct notifier_block *nb, unsigned long val,
 			 void *data)
 {
 	struct sa1100fb_info *fbi = TO_INF(nb, freq_transition);
-	struct cpufreq_freqs *f = data;
 	u_int pcd;
 
 	switch (val) {
@@ -1012,7 +1019,7 @@ sa1100fb_freq_transition(struct notifier_block *nb, unsigned long val,
 		break;
 
 	case CPUFREQ_POSTCHANGE:
-		pcd = get_pcd(fbi->fb.var.pixclock, f->new);
+		pcd = get_pcd(fbi, fbi->fb.var.pixclock);
 		fbi->reg_lccr3 = (fbi->reg_lccr3 & ~0xff) | LCCR3_PixClkDiv(pcd);
 		set_ctrlr_state(fbi, C_ENABLE_CLKCHANGE);
 		break;
@@ -1219,6 +1226,13 @@ static int sa1100fb_probe(struct platform_device *pdev)
 	if (!fbi)
 		goto failed;
 
+	fbi->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(fbi->clk)) {
+		ret = PTR_ERR(fbi->clk);
+		fbi->clk = NULL;
+		goto failed;
+	}
+
 	fbi->base = ioremap(res->start, resource_size(res));
 	if (!fbi->base)
 		goto failed;
@@ -1271,6 +1285,8 @@ static int sa1100fb_probe(struct platform_device *pdev)
  failed:
 	if (fbi)
 		iounmap(fbi->base);
+	if (fbi->clk)
+		clk_put(fbi->clk);
 	kfree(fbi);
 	release_mem_region(res->start, resource_size(res));
 	return ret;
drivers/video/fbdev/sa1100fb.h +1 -0
index fc5d429..0139d13 100644
@@ -68,6 +68,7 @@ struct sa1100fb_info {
 #endif
 
 	const struct sa1100fb_mach_info *inf;
+	struct clk *clk;
 };
 
 #define TO_INF(ptr,member)	container_of(ptr,struct sa1100fb_info,member)
Signature · 1 line
2.1.3
#4 lore raw

PATCH v2 4/6 pcmcia: soc-common: enable/disable socket clocks

In reply to Dmitry Eremin-Solenikov

Call clk_prepare_enable() during hw_init() and clk_disable_unprepare() during hw_shutdown() to ensure that the clock rates returned by clk_get_rate() are correct.

It is safe to call enable/disable functions even on NULL clock, so this patch will not break cases when the socket clock is not set.

---
 drivers/pcmcia/soc_common.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/pcmcia/soc_common.c +4 -0
index a2bc6ee..933f465 100644
@@ -120,6 +120,8 @@ static void __soc_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt,
 
 	if (skt->ops->hw_shutdown)
 		skt->ops->hw_shutdown(skt);
+
+	clk_disable_unprepare(skt->clk);
 }
 
 static void soc_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt)
@@ -131,6 +133,8 @@ static int soc_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
 {
 	int ret = 0, i;
 
+	clk_prepare_enable(skt->clk);
+
 	if (skt->ops->hw_init) {
 		ret = skt->ops->hw_init(skt);
 		if (ret)
Signature · 1 line
2.1.3
#5 lore raw

PATCH v2 5/6 pcmcia: sa1111: provide device clock

In reply to Dmitry Eremin-Solenikov

Both pxa2xx (long ago) and sa1100 (now) make use of clock device to get the cpu speed. Make sa1111 glue code provide clock to platform layer.

---
 drivers/pcmcia/sa1111_generic.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/pcmcia/sa1111_generic.c +7 -0
index 65b02c3..7bae7e5 100644
@@ -145,6 +145,12 @@ int sa1111_pcmcia_add(struct sa1111_dev *dev, struct pcmcia_low_level *ops,
 			return -ENOMEM;
 
 		s->soc.nr = ops->first + i;
+		s->soc.clk = clk_get(&dev->dev, NULL);
+		if (IS_ERR(s->soc.clk)) {
+			ret = PTR_ERR(s->soc.clk);
+			kfree(s);
+			return ret;
+		}
 		soc_pcmcia_init_one(&s->soc, ops, &dev->dev);
 		s->dev = dev;
 		if (s->soc.nr) {
@@ -220,6 +226,7 @@ static int pcmcia_remove(struct sa1111_dev *dev)
 	for (; s; s = next) {
 		next = s->next;
 		soc_pcmcia_remove_one(&s->soc);
+		clk_put(s->soc.clk);
 		kfree(s);
 	}
 
Signature · 1 line
2.1.3
#6 lore raw

PATCH v2 6/6 pcmcia: sa1100: make use of device clock

In reply to Dmitry Eremin-Solenikov

Use per-device clock (instead of calling cpufreq_get(0), which can return 0 if no cpu frequency driver is selected) to program timings.

---
 drivers/pcmcia/sa1100_generic.c |  1 +
 drivers/pcmcia/sa11xx_base.c    | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)
drivers/pcmcia/sa1100_generic.c +1 -0
index ff8a027..d2ab060 100644
@@ -93,6 +93,7 @@ static int sa11x0_drv_pcmcia_remove(struct platform_device *dev)
 	for (i = 0; i < sinfo->nskt; i++)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
 
+	clk_put(sinfo->clk);
 	kfree(sinfo);
 	return 0;
 }
drivers/pcmcia/sa11xx_base.c +12 -2
index 54d3089..cf6de2c 100644
@@ -135,14 +135,16 @@ sa1100_pcmcia_frequency_change(struct soc_pcmcia_socket *skt,
 static int
 sa1100_pcmcia_set_timing(struct soc_pcmcia_socket *skt)
 {
-	return sa1100_pcmcia_set_mecr(skt, cpufreq_get(0));
+	unsigned long clk = clk_get_rate(skt->clk);
+
+	return sa1100_pcmcia_set_mecr(skt, clk / 1000);
 }
 
 static int
 sa1100_pcmcia_show_timing(struct soc_pcmcia_socket *skt, char *buf)
 {
 	struct soc_pcmcia_timing timing;
-	unsigned int clock = cpufreq_get(0);
+	unsigned int clock = clk_get_rate(skt->clk);
 	unsigned long mecr = MECR;
 	char *p = buf;
 
@@ -218,6 +220,11 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 	struct skt_dev_info *sinfo;
 	struct soc_pcmcia_socket *skt;
 	int i, ret = 0;
+	struct clk *clk;
+
+	clk = clk_get(dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
 
 	sa11xx_drv_pcmcia_ops(ops);
 
@@ -226,12 +233,14 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 		return -ENOMEM;
 
 	sinfo->nskt = nr;
+	sinfo->clk = clk;
 
 	/* Initialize processor specific parameters */
 	for (i = 0; i < nr; i++) {
 		skt = &sinfo->skt[i];
 
 		skt->nr = first + i;
+		skt->clk = clk;
 		soc_pcmcia_init_one(skt, ops, dev);
 
 		ret = sa11xx_drv_pcmcia_add_one(skt);
@@ -242,6 +251,7 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 	if (ret) {
 		while (--i >= 0)
 			soc_pcmcia_remove_one(&sinfo->skt[i]);
+		clk_put(clk);
 		kfree(sinfo);
 	} else {
 		dev_set_drvdata(dev, sinfo);
Signature · 1 line
2.1.3