[PATCH 05/10] ARM: OMAP1: Make 770 LCD work

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

Cc: linux-fbdev-devel@lists.sourceforge.net
Signed-off-by: Andrew de Quincey <redacted>
Signed-off-by: Imre Deak <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap1/board-nokia770.c     |   16 ++++++++++++++++
 arch/arm/plat-omap/include/mach/hwa742.h |    5 +----
 drivers/video/omap/hwa742.c              |   26 ++++++++++----------------
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
index 8780ca6..2c4785e 100644
--- a/arch/arm/mach-omap1/board-nokia770.c
+++ b/arch/arm/mach-omap1/board-nokia770.c

@@ -33,6 +33,7 @@
 #include <mach/common.h>
 #include <mach/dsp_common.h>
 #include <mach/omapfb.h>
+#include <mach/hwa742.h>
 #include <mach/lcd_mipid.h>
 #include <mach/mmc.h>
 #include <mach/usb.h>

@@ -163,6 +164,20 @@ static struct spi_board_info nokia770_spi_board_info[] __initdata = {
 	},
 };
 
+static struct hwa742_platform_data nokia770_hwa742_platform_data = {
+	.sys_ck 		= NULL,
+	.te_connected		= 1,
+};
+
+static void hwa742_dev_init(void)
+{
+	nokia770_hwa742_platform_data.sys_ck = clk_get(NULL, "bclk");
+	if (IS_ERR(nokia770_hwa742_platform_data.sys_ck)) {
+		printk(KERN_ERR "can't get HWA742 clock\n");
+	} else {
+		omapfb_set_ctrl_platform_data(&nokia770_hwa742_platform_data);
+	}
+}
 
 /* assume no Mini-AB port */
 

@@ -371,6 +386,7 @@ static void __init omap_nokia770_init(void)
 	omap_serial_init();
 	omap_register_i2c_bus(1, 100, NULL, 0);
 	omap_dsp_init();
+	hwa742_dev_init();
 	ads7846_dev_init();
 	mipid_dev_init();
 	omap_usb_init(&nokia770_usb_config);

diff --git a/arch/arm/plat-omap/include/mach/hwa742.h b/arch/arm/plat-omap/include/mach/hwa742.h
index 577f492..c00e05d 100644
--- a/arch/arm/plat-omap/include/mach/hwa742.h
+++ b/arch/arm/plat-omap/include/mach/hwa742.h

@@ -2,10 +2,7 @@
 #define _HWA742_H
 
 struct hwa742_platform_data {
-	void		(*power_up)(struct device *dev);
-	void		(*power_down)(struct device *dev);
-	unsigned long	(*get_clock_rate)(struct device *dev);
-
+	struct clk 	*sys_ck;
 	unsigned	te_connected:1;
 };
 

diff --git a/drivers/video/omap/hwa742.c b/drivers/video/omap/hwa742.c
index 8aa6e47..1230476 100644
--- a/drivers/video/omap/hwa742.c
+++ b/drivers/video/omap/hwa742.c

@@ -133,8 +133,7 @@ struct {
 	struct lcd_ctrl_extif	*extif;
 	struct lcd_ctrl		*int_ctrl;
 
-	void			(*power_up)(struct device *dev);
-	void			(*power_down)(struct device *dev);
+	struct clk		*sys_ck;
 } hwa742;
 
 struct lcd_ctrl hwa742_ctrl;

@@ -915,14 +914,13 @@ static void hwa742_suspend(void)
 	hwa742_set_update_mode(OMAPFB_UPDATE_DISABLED);
 	/* Enable sleep mode */
 	hwa742_write_reg(HWA742_POWER_SAVE, 1 << 1);
-	if (hwa742.power_down != NULL)
-		hwa742.power_down(hwa742.fbdev->dev);
+	clk_disable(hwa742.sys_ck);
 }
 
 static void hwa742_resume(void)
 {
-	if (hwa742.power_up != NULL)
-		hwa742.power_up(hwa742.fbdev->dev);
+	clk_enable(hwa742.sys_ck);
+
 	/* Disable sleep mode */
 	hwa742_write_reg(HWA742_POWER_SAVE, 0);
 	while (1) {

@@ -955,14 +953,13 @@ static int hwa742_init(struct omapfb_device *fbdev, int ext_mode,
 	omapfb_conf = fbdev->dev->platform_data;
 	ctrl_conf = omapfb_conf->ctrl_platform_data;
 
-	if (ctrl_conf == NULL || ctrl_conf->get_clock_rate == NULL) {
+	if (ctrl_conf == NULL || ctrl_conf->sys_ck == NULL) {
 		dev_err(fbdev->dev, "HWA742: missing platform data\n");
 		r = -ENOENT;
 		goto err1;
 	}
 
-	hwa742.power_down = ctrl_conf->power_down;
-	hwa742.power_up = ctrl_conf->power_up;
+	hwa742.sys_ck = ctrl_conf->sys_ck;
 
 	spin_lock_init(&hwa742.req_lock);
 

@@ -972,12 +969,11 @@ static int hwa742_init(struct omapfb_device *fbdev, int ext_mode,
 	if ((r = hwa742.extif->init(fbdev)) < 0)
 		goto err2;
 
-	ext_clk = ctrl_conf->get_clock_rate(fbdev->dev);
+	ext_clk = clk_get_rate(hwa742.sys_ck);
 	if ((r = calc_extif_timings(ext_clk, &extif_mem_div)) < 0)
 		goto err3;
 	hwa742.extif->set_timings(&hwa742.reg_timings);
-	if (hwa742.power_up != NULL)
-		hwa742.power_up(fbdev->dev);
+	clk_enable(hwa742.sys_ck);
 
 	calc_hwa742_clk_rates(ext_clk, &sys_clk, &pix_clk);
 	if ((r = calc_extif_timings(sys_clk, &extif_mem_div)) < 0)

@@ -1040,8 +1036,7 @@ static int hwa742_init(struct omapfb_device *fbdev, int ext_mode,
 
 	return 0;
 err4:
-	if (hwa742.power_down != NULL)
-		hwa742.power_down(fbdev->dev);
+	clk_disable(hwa742.sys_ck);
 err3:
 	hwa742.extif->cleanup();
 err2:

@@ -1055,8 +1050,7 @@ static void hwa742_cleanup(void)
 	hwa742_set_update_mode(OMAPFB_UPDATE_DISABLED);
 	hwa742.extif->cleanup();
 	hwa742.int_ctrl->cleanup();
-	if (hwa742.power_down != NULL)
-		hwa742.power_down(hwa742.fbdev->dev);
+	clk_disable(hwa742.sys_ck);
 }
 
 struct lcd_ctrl hwa742_ctrl = {

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Quoting Russell King - ARM Linux [off-list ref]:

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

quoted

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Interesting idea - I now have a prototype patch implementing this, which I'll send when I get home (otherwise it would be an "it compiles" "test" :)

There isn't a common definition of clk_add_alias() though - that symbol is defined in arch/arm/mach-pxa/clock.c, so I had to copy it into arch/arm/plat-omap/clock.c

Quoting Andrew de Quincey [off-list ref]:

Quoting Russell King - ARM Linux [off-list ref]:

quoted

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

quoted

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Interesting idea - I now have a prototype patch implementing this, which I'll send when I get home (otherwise it would be an "it compiles" "test" :)

There isn't a common definition of clk_add_alias() though - that symbol is defined in arch/arm/mach-pxa/clock.c, so I had to copy it into arch/arm/plat-omap/clock.c

Attached is a patch implementing this method. I don't like the duplication of the clk_alias code, but I imagine that could be resolved if this method was chosen....

* Andrew de Quincey [off-list ref] [090525 09:41]:

Quoting Andrew de Quincey [off-list ref]:

quoted

Quoting Russell King - ARM Linux [off-list ref]:

quoted

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

quoted

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Interesting idea - I now have a prototype patch implementing this,  
which I'll send when I get home (otherwise it would be an "it  
compiles" "test" :)

There isn't a common definition of clk_add_alias() though - that  
symbol is defined in arch/arm/mach-pxa/clock.c, so I had to copy it  
into arch/arm/plat-omap/clock.c

Attached is a patch implementing this method. I don't like the  
duplication of the clk_alias code, but I imagine that could be resolved 
if this method was chosen....

Andrew, can you please reply with your Signed-off-by?

Thanks,

Tony

Quoting Tony Lindgren [off-list ref]:

* Andrew de Quincey [off-list ref] [090525 09:41]:

quoted

Quoting Andrew de Quincey [off-list ref]:

quoted

Quoting Russell King - ARM Linux [off-list ref]:

quoted

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

quoted

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Interesting idea - I now have a prototype patch implementing this,
which I'll send when I get home (otherwise it would be an "it
compiles" "test" :)

There isn't a common definition of clk_add_alias() though - that
symbol is defined in arch/arm/mach-pxa/clock.c, so I had to copy it
into arch/arm/plat-omap/clock.c

Attached is a patch implementing this method. I don't like the
duplication of the clk_alias code, but I imagine that could be resolved
if this method was chosen....

Andrew, can you please reply with your Signed-off-by?

Sure - sorry I was holding off in case there were any comments on the method...

Signed-off-by: Andrew de Quincey <redacted>

* Andrew de Quincey [off-list ref] [090528 11:44]:

Quoting Tony Lindgren [off-list ref]:

quoted

* Andrew de Quincey [off-list ref] [090525 09:41]:

quoted

Quoting Andrew de Quincey [off-list ref]:

quoted

Quoting Russell King - ARM Linux [off-list ref]:

quoted

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

quoted

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Interesting idea - I now have a prototype patch implementing this,
which I'll send when I get home (otherwise it would be an "it
compiles" "test" :)

There isn't a common definition of clk_add_alias() though - that
symbol is defined in arch/arm/mach-pxa/clock.c, so I had to copy it
into arch/arm/plat-omap/clock.c

Attached is a patch implementing this method. I don't like the
duplication of the clk_alias code, but I imagine that could be resolved
if this method was chosen....

Andrew, can you please reply with your Signed-off-by?

Sure - sorry I was holding off in case there were any comments on the  
method...

I added a note about maybe moving the clk_add_alias() to
arch/arm/common/clkdev.c later.

Also noticed there were some whitespace errors, updated version below.

Regards,

Tony

Signed-off-by: Andrew de Quincey <redacted>

On Thu, May 28, 2009 at 11:20:48AM -0700, Tony Lindgren wrote:

* Andrew de Quincey [off-list ref] [090525 09:41]:

quoted

Quoting Andrew de Quincey [off-list ref]:

quoted

Quoting Russell King - ARM Linux [off-list ref]:

quoted

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

quoted

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Interesting idea - I now have a prototype patch implementing this,  
which I'll send when I get home (otherwise it would be an "it  
compiles" "test" :)

There isn't a common definition of clk_add_alias() though - that  
symbol is defined in arch/arm/mach-pxa/clock.c, so I had to copy it  
into arch/arm/plat-omap/clock.c

Attached is a patch implementing this method. I don't like the  
duplication of the clk_alias code, but I imagine that could be resolved 
if this method was chosen....

Andrew, can you please reply with your Signed-off-by?

Actually, I'm very tempted to suggest that clk_add_alias() should go
into arch/arm/common/clkdev.c if we're going to have more people use it.

On Thu, May 28, 2009 at 11:20:48AM -0700, Tony Lindgren wrote:

quoted

+int clk_add_alias(const char *alias, const char *alias_dev_name, char *id,
+	struct device *dev)
+{
+	struct clk *r = clk_get(dev, id);
+	struct clk_lookup *l;
+
+	if (!r)
+		return -ENODEV;
+
+	l = clkdev_alloc(r, alias, alias_dev_name);
+	clk_put(r);
+	if (!l)
+		return -ENODEV;
+	clkdev_add(l);
+	return 0;
+}
+EXPORT_SYMBOL(clk_add_alias);

Oh, and a really good thing to do would be to fix the error checking and
returning in there (why did I miss it in the original PXA version...)

* Russell King - ARM Linux [off-list ref] [090528 12:53]:

On Thu, May 28, 2009 at 11:20:48AM -0700, Tony Lindgren wrote:

quoted

quoted

+int clk_add_alias(const char *alias, const char *alias_dev_name, char *id,
+	struct device *dev)
+{
+	struct clk *r = clk_get(dev, id);
+	struct clk_lookup *l;
+
+	if (!r)
+		return -ENODEV;
+
+	l = clkdev_alloc(r, alias, alias_dev_name);
+	clk_put(r);
+	if (!l)
+		return -ENODEV;
+	clkdev_add(l);
+	return 0;
+}
+EXPORT_SYMBOL(clk_add_alias);

Oh, and a really good thing to do would be to fix the error checking and
returning in there (why did I miss it in the original PXA version...)

How about this? The prototype is in clk.h now, is that OK?

Tony

* Tony Lindgren [off-list ref] [090528 12:12]:

* Andrew de Quincey [off-list ref] [090528 11:44]:

quoted

Quoting Tony Lindgren [off-list ref]:

quoted

* Andrew de Quincey [off-list ref] [090525 09:41]:

quoted

Quoting Andrew de Quincey [off-list ref]:

quoted

Quoting Russell King - ARM Linux [off-list ref]:

quoted

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

quoted

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Interesting idea - I now have a prototype patch implementing this,
which I'll send when I get home (otherwise it would be an "it
compiles" "test" :)

There isn't a common definition of clk_add_alias() though - that
symbol is defined in arch/arm/mach-pxa/clock.c, so I had to copy it
into arch/arm/plat-omap/clock.c

Attached is a patch implementing this method. I don't like the
duplication of the clk_alias code, but I imagine that could be resolved
if this method was chosen....

Andrew, can you please reply with your Signed-off-by?

Sure - sorry I was holding off in case there were any comments on the  
method...

I added a note about maybe moving the clk_add_alias() to
arch/arm/common/clkdev.c later.

Also noticed there were some whitespace errors, updated version below.

One more time with clk_add_alias() in a separate patch.

Tony

Quoting Russell King - ARM Linux [off-list ref]:

On Thu, May 28, 2009 at 11:20:48AM -0700, Tony Lindgren wrote:

quoted

* Andrew de Quincey [off-list ref] [090525 09:41]:

quoted

Quoting Andrew de Quincey [off-list ref]:

quoted

Quoting Russell King - ARM Linux [off-list ref]:

quoted

On Tue, May 19, 2009 at 04:37:32PM -0700, Tony Lindgren wrote:

quoted

Make 770 LCD work by passing the clock from platform data.
Also remove the old unused functions.

I don't like this - because its passing struct clk's through platform
data.  That's not how things are supposed to work.

The way PXA solves this problem is to have clock aliases - see
clk_add_alias().

Interesting idea - I now have a prototype patch implementing this,
which I'll send when I get home (otherwise it would be an "it
compiles" "test" :)

There isn't a common definition of clk_add_alias() though - that
symbol is defined in arch/arm/mach-pxa/clock.c, so I had to copy it
into arch/arm/plat-omap/clock.c

Attached is a patch implementing this method. I don't like the
duplication of the clk_alias code, but I imagine that could be resolved
if this method was chosen....

Andrew, can you please reply with your Signed-off-by?

Actually, I'm very tempted to suggest that clk_add_alias() should go
into arch/arm/common/clkdev.c if we're going to have more people use it.

Cool!

Out of interest, what is the best practice way to pass platform specific GPIOs around? There are a number of other n770 related drivers I'm intending on cleaning up, several of which have GPIO numbers hardcoded in them. Should these be passed in platform structures, or is there something analogous to the clk_alias for GPIOs?

Andrew de Quincey [off-list ref] writes:

Out of interest, what is the best practice way to pass platform
specific GPIOs around? There are a number of other n770 related
drivers I'm intending on cleaning up, several of which have GPIO
numbers hardcoded in them. Should these be passed in platform
structures, or is there something analogous to the clk_alias for
GPIOs?

I was told that platform structures are the preferred way. For wl12xx (a
wi-fi driver) I created a set_power function pointer which will control
the power gpio line:

27 struct wl12xx_platform_data {
28         void (*set_power)(bool enable);
29 };

http://git.kernel.org/?p=linux/kernel/git/linville/wireless-testing.git;a=blob;f=include/linux/spi/wl12xx.h;h=11430cab2aad71242946ebfc7c0ce778d8bf26a1;hb=HEAD

And the irq is configured in the board file and provided to wl12xx with
struct spi_device.irq. So wl12xx doesn't use gpio code at all, it's all
done in the board file. 

Please comment if this isn't the correct way to do this.

-- 
Kalle Valo

* Tony Lindgren [off-list ref] [090528 14:05]:

* Russell King - ARM Linux [off-list ref] [090528 12:53]:

quoted

On Thu, May 28, 2009 at 11:20:48AM -0700, Tony Lindgren wrote:

quoted

quoted

+int clk_add_alias(const char *alias, const char *alias_dev_name, char *id,
+	struct device *dev)
+{
+	struct clk *r = clk_get(dev, id);
+	struct clk_lookup *l;
+
+	if (!r)
+		return -ENODEV;
+
+	l = clkdev_alloc(r, alias, alias_dev_name);
+	clk_put(r);
+	if (!l)
+		return -ENODEV;
+	clkdev_add(l);
+	return 0;
+}
+EXPORT_SYMBOL(clk_add_alias);

Oh, and a really good thing to do would be to fix the error checking and
returning in there (why did I miss it in the original PXA version...)

How about this? The prototype is in clk.h now, is that OK?

Added to patch tracking as 5536/1.
 

Tony