rfc: amba_pl022: LCD and AMBA PCLK clock updates

rfc: amba_pl022: LCD and AMBA PCLK clock updates

[wellsk40 at gmail.com]

This patch series fixes unbalanced clock enables and disables for the
pl11x driver and adds support for the AMBA PCLK via the AMBA bus driver.

This patch series depends on the AMBA PCLK patch..

The LPC32xx will thrown an ARM exception if an attempt is made to access a
peripheral register set without the clocks enabled. The LCD clock was not
enabled until a number of register accesses already occured. Because this
clock wasn't enabled, the driver wouldn't work correctly. This is fixed
with the AMBA PCLK patch.

The clock inbalance occurred during when no clocks were yet enabled and
the clcdfb_set_par() function was called during probe. It would attempt to
disable the inactive clock in clcdfb_disable(). You can also force the
inbalance by writing to /sys/class/graphics/blank. This is fixed by
preventing calls to the clk_* functions if the clock is already in the
desired state.

The LCD peripheral and AMBA PCLK are treated as 2 different clocks in the
patch. Prior to any register access, the AMBA PCLK is enabled. It's also
disabled after the access. During probe, the AMBA PCLK clock is assumed to
be initially active. Prior the leaving probe, the AMBA PLCK is disabled.
Prior to leaving the disable funciton, the AMBA clock is enabled (to be
disabled in the AMBA bus driver). Some functions will enable and disable
the AMBA PCLK if it's already enabled. These nested clock enables/disable
operations are balanced.

[PATCH 1/2] amba_pl022: Fix driver clock enable/disable balance issues

[wellsk40 at gmail.com]

From: Kevin Wells <wellsk40@gmail.com>

The pl022 clock enable and disable calls can become unbalanced. This
code prevents clk_disable() from being called until clk_enable()
has been called first.

Note the LCP32xx arch (and probably other archs too) requires clocks
for the AMBA peripheral enabled prior to any register access. This
patch alone won't prevent register access without an active clock.
---
 drivers/video/amba-clcd.c |   10 ++++++++--
 include/linux/amba/clcd.h |    1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
index afe21e6..1c2c683 100644
--- a/drivers/video/amba-clcd.c
+++ b/drivers/video/amba-clcd.c
@@ -80,7 +80,10 @@ static void clcdfb_disable(struct clcd_fb *fb)
 	/*
 	 * Disable CLCD clock source.
 	 */
-	clk_disable(fb->clk);
+	if (fb->clk_enabled) {
+		fb->clk_enabled = false;
+		clk_disable(fb->clk);
+	}
 }
 
 static void clcdfb_enable(struct clcd_fb *fb, u32 cntl)
@@ -88,7 +91,10 @@ static void clcdfb_enable(struct clcd_fb *fb, u32 cntl)
 	/*
 	 * Enable the CLCD clock source.
 	 */
-	clk_enable(fb->clk);
+	if (!fb->clk_enabled) {
+		fb->clk_enabled = true;
+		clk_enable(fb->clk);
+	}
 
 	/*
 	 * Bring up by first enabling..
diff --git a/include/linux/amba/clcd.h b/include/linux/amba/clcd.h
index ca16c38..7bbf26a 100644
--- a/include/linux/amba/clcd.h
+++ b/include/linux/amba/clcd.h
@@ -142,6 +142,7 @@ struct clcd_fb {
 	struct fb_info		fb;
 	struct amba_device	*dev;
 	struct clk		*clk;
+	bool			clk_enabled;
 	struct clcd_panel	*panel;
 	struct clcd_board	*board;
 	void			*board_data;
-- 
1.7.1.1

[PATCH 1/2] amba_pl022: Fix driver clock enable/disable balance issues

[Linus Walleij]

2010/8/3  <wellsk40@gmail.com>:

> The pl022 clock enable and disable calls can become unbalanced. This
> code prevents clk_disable() from being called until clk_enable()
> has been called first.

This is not for the PL022. It's for the CLCD (Is that PL220?)
Same with the other patch...

Yours,
Linus Walleij

[PATCH 1/2] amba_pl022: Fix driver clock enable/disable balance issues

[Kevin Wells]

> 
> > The pl022 clock enable and disable calls can become unbalanced. This
> > code prevents clk_disable() from being called until clk_enable()
> > has been called first.
> 
> This is not for the PL022. It's for the CLCD (Is that PL220?)
> Same with the other patch...
> 

Sorry about the confusion there. Tuesday was a bit hectic!
I'll repost the series with the correct info.

kevin

[PATCH 2/2] amba_pl022: Add support for amba peripheral clocking

[wellsk40 at gmail.com]

From: Kevin Wells <wellsk40@gmail.com>

For register based accesses, enable the AMBA peripheral clock
prior to a register access and disable it after a register access.
The AMBA PCLK is disabled on probe and restored on remove.
---
 drivers/video/amba-clcd.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
index 1c2c683..1d2fe32 100644
--- a/drivers/video/amba-clcd.c
+++ b/drivers/video/amba-clcd.c
@@ -65,6 +65,8 @@ static void clcdfb_disable(struct clcd_fb *fb)
 	if (fb->board->disable)
 		fb->board->disable(fb);
 
+	amba_pclk_enable(fb->dev);
+
 	val = readl(fb->regs + fb->off_cntl);
 	if (val & CNTL_LCDPWR) {
 		val &= ~CNTL_LCDPWR;
@@ -77,6 +79,8 @@ static void clcdfb_disable(struct clcd_fb *fb)
 		writel(val, fb->regs + fb->off_cntl);
 	}
 
+	amba_pclk_disable(fb->dev);
+
 	/*
 	 * Disable CLCD clock source.
 	 */
@@ -96,6 +100,8 @@ static void clcdfb_enable(struct clcd_fb *fb, u32 cntl)
 		clk_enable(fb->clk);
 	}
 
+	amba_pclk_enable(fb->dev);
+
 	/*
 	 * Bring up by first enabling..
 	 */
@@ -110,6 +116,8 @@ static void clcdfb_enable(struct clcd_fb *fb, u32 cntl)
 	cntl |= CNTL_LCDPWR;
 	writel(cntl, fb->regs + fb->off_cntl);
 
+	amba_pclk_disable(fb->dev);
+
 	/*
 	 * finally, enable the interface.
 	 */
@@ -217,6 +225,7 @@ static int clcdfb_set_par(struct fb_info *info)
 	fb->board->decode(fb, &regs);
 
 	clcdfb_disable(fb);
+	amba_pclk_disable(fb->dev);
 
 	writel(regs.tim0, fb->regs + CLCD_TIM0);
 	writel(regs.tim1, fb->regs + CLCD_TIM1);
@@ -242,6 +251,8 @@ static int clcdfb_set_par(struct fb_info *info)
 		readl(fb->regs + fb->off_ienb), readl(fb->regs + fb->off_cntl));
 #endif
 
+	amba_pclk_disable(fb->dev);
+
 	return 0;
 }
 
@@ -290,8 +301,10 @@ clcdfb_setcolreg(unsigned int regno, unsigned int red, unsigned int green,
 			mask = 0xffff0000;
 		}
 
+		amba_pclk_enable(fb->dev);
 		val = readl(fb->regs + hw_reg) & mask;
 		writel(val | newval, fb->regs + hw_reg);
+		amba_pclk_disable(fb->dev);
 	}
 
 	return regno > 255;
@@ -492,6 +505,9 @@ static int clcdfb_probe(struct amba_device *dev, struct amba_id *id)
 
 	ret = clcdfb_register(fb); 
 	if (ret == 0) {
+		/* Disable AMBA PCLK */
+		amba_pclk_disable(dev);
+
 		amba_set_drvdata(dev, fb);
 		goto out;
 	}
@@ -524,6 +540,9 @@ static int clcdfb_remove(struct amba_device *dev)
 
 	amba_release_regions(dev);
 
+	/* Restore AMBA PCLK */
+	amba_pclk_enable(dev);
+
 	return 0;
 }
 
-- 
1.7.1.1

rfc: amba_pl022: LCD and AMBA PCLK clock updates

[Linus Walleij]

2010/8/3  <wellsk40@gmail.com>:

> This patch series fixes unbalanced clock enables and disables for the
> pl11x driver and adds support for the AMBA PCLK via the AMBA bus driver.

I don't understand one thing of this, the subject says it's about PL022 (SPI),
this comment says it's for PL011 (serial) and then the patch is for CLCD...

I guess it's for the CLCD!

:-)

> This patch series depends on the AMBA PCLK patch..

That thing is already merged to the mainline kernel so no problem.

Yours,
Linus Walleij