public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Temporary fix to boot OMAP1 based boards
@ 2008-09-23 15:23 Arun KS
  2008-09-23 22:43 ` Felipe Balbi
  2008-09-24  3:51 ` Hiroshi DOYU
  0 siblings, 2 replies; 16+ messages in thread
From: Arun KS @ 2008-09-23 15:23 UTC (permalink / raw)
  To: linux-omap@vger.kernel.org

This patch is a temporary fix for a kernel panic causing at omap1_clk_init

Signed-off-by: Arun KS <arunks@mistralsolutions.com>
---
 arch/arm/plat-omap/include/mach/clock.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/mach/clock.h
b/arch/arm/plat-omap/include/mach/clock.h
index f26ad8c..b9c471e 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -67,7 +67,11 @@ struct clk {
 	struct clk		*parent;
 	unsigned long		rate;
 	__u32			flags;
+#if defined(CONFIG_ARCH_OMAP1)
+	void __iomem            *enable_reg;
+#elif
 	u16			enable_reg;
+#endif
 	__u8			enable_bit;
 	__s8			usecount;
 	u8			idlest_bit;
-- 
1.5.3.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-23 15:23 [PATCH] Temporary fix to boot OMAP1 based boards Arun KS
@ 2008-09-23 22:43 ` Felipe Balbi
  2008-09-24  4:50   ` Arun KS
  2008-09-24  3:51 ` Hiroshi DOYU
  1 sibling, 1 reply; 16+ messages in thread
From: Felipe Balbi @ 2008-09-23 22:43 UTC (permalink / raw)
  To: Arun KS; +Cc: linux-omap@vger.kernel.org

On Tue, Sep 23, 2008 at 08:53:09PM +0530, Arun KS wrote:
> This patch is a temporary fix for a kernel panic causing at omap1_clk_init
> 
> Signed-off-by: Arun KS <arunks@mistralsolutions.com>
> ---
>  arch/arm/plat-omap/include/mach/clock.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/mach/clock.h
> b/arch/arm/plat-omap/include/mach/clock.h
> index f26ad8c..b9c471e 100644
> --- a/arch/arm/plat-omap/include/mach/clock.h
> +++ b/arch/arm/plat-omap/include/mach/clock.h
> @@ -67,7 +67,11 @@ struct clk {
>  	struct clk		*parent;
>  	unsigned long		rate;
>  	__u32			flags;
> +#if defined(CONFIG_ARCH_OMAP1)
> +	void __iomem            *enable_reg;
> +#elif
>  	u16			enable_reg;
> +#endif

Did you try removing those (void __iomem *) casts from
arch/arm/mach-omap1/clock.h ??

with vim you can fix it by:

:%s/(void\ __iomem\ \*)//g

-- 
balbi

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-23 15:23 [PATCH] Temporary fix to boot OMAP1 based boards Arun KS
  2008-09-23 22:43 ` Felipe Balbi
@ 2008-09-24  3:51 ` Hiroshi DOYU
  2008-09-24  7:05   ` Arun KS
  1 sibling, 1 reply; 16+ messages in thread
From: Hiroshi DOYU @ 2008-09-24  3:51 UTC (permalink / raw)
  To: arunks, Paul Walmsley; +Cc: linux-omap

Hi,

How about introducing a new typedef in order to accommodate the
different size of enable_regs without ifdefs?

I think that this is apparently the case categorized into (a) in
'CodyingStyle'.

     Hiroshi DOYU

From: "ext Arun KS" <arunks@mistralsolutions.com>
Subject: [PATCH] Temporary fix to boot OMAP1 based boards
Date: Tue, 23 Sep 2008 20:53:09 +0530

> This patch is a temporary fix for a kernel panic causing at omap1_clk_init
> 
> Signed-off-by: Arun KS <arunks@mistralsolutions.com>
> ---
>  arch/arm/plat-omap/include/mach/clock.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/mach/clock.h
> b/arch/arm/plat-omap/include/mach/clock.h
> index f26ad8c..b9c471e 100644
> --- a/arch/arm/plat-omap/include/mach/clock.h
> +++ b/arch/arm/plat-omap/include/mach/clock.h
> @@ -67,7 +67,11 @@ struct clk {
>  	struct clk		*parent;
>  	unsigned long		rate;
>  	__u32			flags;
> +#if defined(CONFIG_ARCH_OMAP1)
> +	void __iomem            *enable_reg;
> +#elif
>  	u16			enable_reg;
> +#endif
>  	__u8			enable_bit;
>  	__s8			usecount;
>  	u8			idlest_bit;
> -- 
> 1.5.3.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-23 22:43 ` Felipe Balbi
@ 2008-09-24  4:50   ` Arun KS
  0 siblings, 0 replies; 16+ messages in thread
From: Arun KS @ 2008-09-24  4:50 UTC (permalink / raw)
  To: me; +Cc: linux-omap@vger.kernel.org

On Wed, Sep 24, 2008 at 4:13 AM, Felipe Balbi <me@felipebalbi.com> wrote:
> On Tue, Sep 23, 2008 at 08:53:09PM +0530, Arun KS wrote:
>> This patch is a temporary fix for a kernel panic causing at omap1_clk_init
>>
>> Signed-off-by: Arun KS <arunks@mistralsolutions.com>
>> ---
>>  arch/arm/plat-omap/include/mach/clock.h |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/mach/clock.h
>> b/arch/arm/plat-omap/include/mach/clock.h
>> index f26ad8c..b9c471e 100644
>> --- a/arch/arm/plat-omap/include/mach/clock.h
>> +++ b/arch/arm/plat-omap/include/mach/clock.h
>> @@ -67,7 +67,11 @@ struct clk {
>>       struct clk              *parent;
>>       unsigned long           rate;
>>       __u32                   flags;
>> +#if defined(CONFIG_ARCH_OMAP1)
>> +     void __iomem            *enable_reg;
>> +#elif
>>       u16                     enable_reg;
>> +#endif
>
> Did you try removing those (void __iomem *) casts from
> arch/arm/mach-omap1/clock.h ??
>
> with vim you can fix it by:
>
> :%s/(void\ __iomem\ \*)//g
>

This is not working, still panic.

enable_reg variable is u16 now(which is offset). clock.c file of omap2
is modified to support this, but clock.c of omap1 is not.


> --
> balbi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  3:51 ` Hiroshi DOYU
@ 2008-09-24  7:05   ` Arun KS
  2008-09-24  7:12     ` Tony Lindgren
  0 siblings, 1 reply; 16+ messages in thread
From: Arun KS @ 2008-09-24  7:05 UTC (permalink / raw)
  To: Hiroshi DOYU; +Cc: Paul Walmsley, linux-omap

On Wed, Sep 24, 2008 at 9:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Hi,
>
> How about introducing a new typedef in order to accommodate the
> different size of enable_regs without ifdefs?
>
> I think that this is apparently the case categorized into (a) in
> 'CodyingStyle'.

I am not sure which is the right way to do. Is this what you meant?


arch/arm/plat-omap/include/mach/clock.h |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/include/mach/clock.h
b/arch/arm/plat-omap/include/mach/clock.h
index f26ad8c..122532e 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -59,6 +59,12 @@ struct dpll_data {

 #endif

+#if defined(CONFIG_ARCH_OMAP1)
+typedef	void __iomem *reg_t;
+#elif
+typedef	u16	reg_t;
+#endif
+
 struct clk {
 	struct list_head	node;
 	struct module		*owner;
@@ -67,7 +73,7 @@ struct clk {
 	struct clk		*parent;
 	unsigned long		rate;
 	__u32			flags;
-	u16			enable_reg;
+	reg_t			enable_reg;
 	__u8			enable_bit;
 	__s8			usecount;
 	u8			idlest_bit;

Regards
Arun
>
>     Hiroshi DOYU
>
> From: "ext Arun KS" <arunks@mistralsolutions.com>
> Subject: [PATCH] Temporary fix to boot OMAP1 based boards
> Date: Tue, 23 Sep 2008 20:53:09 +0530
>
>> This patch is a temporary fix for a kernel panic causing at omap1_clk_init
>>
>> Signed-off-by: Arun KS <arunks@mistralsolutions.com>
>> ---
>>  arch/arm/plat-omap/include/mach/clock.h |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/mach/clock.h
>> b/arch/arm/plat-omap/include/mach/clock.h
>> index f26ad8c..b9c471e 100644
>> --- a/arch/arm/plat-omap/include/mach/clock.h
>> +++ b/arch/arm/plat-omap/include/mach/clock.h
>> @@ -67,7 +67,11 @@ struct clk {
>>       struct clk              *parent;
>>       unsigned long           rate;
>>       __u32                   flags;
>> +#if defined(CONFIG_ARCH_OMAP1)
>> +     void __iomem            *enable_reg;
>> +#elif
>>       u16                     enable_reg;
>> +#endif
>>       __u8                    enable_bit;
>>       __s8                    usecount;
>>       u8                      idlest_bit;
>> --
>> 1.5.3.4
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  7:05   ` Arun KS
@ 2008-09-24  7:12     ` Tony Lindgren
  2008-09-24  8:00       ` Tony Lindgren
  0 siblings, 1 reply; 16+ messages in thread
From: Tony Lindgren @ 2008-09-24  7:12 UTC (permalink / raw)
  To: Arun KS; +Cc: Hiroshi DOYU, Paul Walmsley, linux-omap

* Arun KS <arunks@mistralsolutions.com> [080924 10:05]:
> On Wed, Sep 24, 2008 at 9:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > Hi,
> >
> > How about introducing a new typedef in order to accommodate the
> > different size of enable_regs without ifdefs?
> >
> > I think that this is apparently the case categorized into (a) in
> > 'CodyingStyle'.
> 
> I am not sure which is the right way to do. Is this what you meant?

No, let's not do this. It should be void __iomem * for sure. Most of
the omap1 clock registers looks just fine for void __iomem *, let's
figure out which registers are broken.

Tony

> 
> 
> arch/arm/plat-omap/include/mach/clock.h |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/mach/clock.h
> b/arch/arm/plat-omap/include/mach/clock.h
> index f26ad8c..122532e 100644
> --- a/arch/arm/plat-omap/include/mach/clock.h
> +++ b/arch/arm/plat-omap/include/mach/clock.h
> @@ -59,6 +59,12 @@ struct dpll_data {
> 
>  #endif
> 
> +#if defined(CONFIG_ARCH_OMAP1)
> +typedef	void __iomem *reg_t;
> +#elif
> +typedef	u16	reg_t;
> +#endif
> +
>  struct clk {
>  	struct list_head	node;
>  	struct module		*owner;
> @@ -67,7 +73,7 @@ struct clk {
>  	struct clk		*parent;
>  	unsigned long		rate;
>  	__u32			flags;
> -	u16			enable_reg;
> +	reg_t			enable_reg;
>  	__u8			enable_bit;
>  	__s8			usecount;
>  	u8			idlest_bit;
> 
> Regards
> Arun
> >
> >     Hiroshi DOYU
> >
> > From: "ext Arun KS" <arunks@mistralsolutions.com>
> > Subject: [PATCH] Temporary fix to boot OMAP1 based boards
> > Date: Tue, 23 Sep 2008 20:53:09 +0530
> >
> >> This patch is a temporary fix for a kernel panic causing at omap1_clk_init
> >>
> >> Signed-off-by: Arun KS <arunks@mistralsolutions.com>
> >> ---
> >>  arch/arm/plat-omap/include/mach/clock.h |    4 ++++
> >>  1 files changed, 4 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/plat-omap/include/mach/clock.h
> >> b/arch/arm/plat-omap/include/mach/clock.h
> >> index f26ad8c..b9c471e 100644
> >> --- a/arch/arm/plat-omap/include/mach/clock.h
> >> +++ b/arch/arm/plat-omap/include/mach/clock.h
> >> @@ -67,7 +67,11 @@ struct clk {
> >>       struct clk              *parent;
> >>       unsigned long           rate;
> >>       __u32                   flags;
> >> +#if defined(CONFIG_ARCH_OMAP1)
> >> +     void __iomem            *enable_reg;
> >> +#elif
> >>       u16                     enable_reg;
> >> +#endif
> >>       __u8                    enable_bit;
> >>       __s8                    usecount;
> >>       u8                      idlest_bit;
> >> --
> >> 1.5.3.4
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  7:12     ` Tony Lindgren
@ 2008-09-24  8:00       ` Tony Lindgren
  2008-09-24  8:03         ` Paul Walmsley
  0 siblings, 1 reply; 16+ messages in thread
From: Tony Lindgren @ 2008-09-24  8:00 UTC (permalink / raw)
  To: Arun KS; +Cc: Hiroshi DOYU, Paul Walmsley, linux-omap

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

* Tony Lindgren <tony@atomide.com> [080924 10:12]:
> * Arun KS <arunks@mistralsolutions.com> [080924 10:05]:
> > On Wed, Sep 24, 2008 at 9:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > > Hi,
> > >
> > > How about introducing a new typedef in order to accommodate the
> > > different size of enable_regs without ifdefs?
> > >
> > > I think that this is apparently the case categorized into (a) in
> > > 'CodyingStyle'.
> > 
> > I am not sure which is the right way to do. Is this what you meant?
> 
> No, let's not do this. It should be void __iomem * for sure. Most of
> the omap1 clock registers looks just fine for void __iomem *, let's
> figure out which registers are broken.

I've pushed a fix for this. Turns out I missed some conversions to
void __iomem * while merging code from arm-devel branch. Fix also
attached.

Can you please check that it works for you now?

Tony

[-- Attachment #2: omap1-clock-fix.patch --]
[-- Type: text/x-diff, Size: 1664 bytes --]

>From 3fa3f8c8f56d0013b3802319c601581d73882ff4 Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Wed, 24 Sep 2008 10:56:18 +0300
Subject: [PATCH] Fix booting of omap1

This got broken because of mismerge from arm-devel.

Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index f26ad8c..24552b2 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -31,7 +31,7 @@ struct clksel {
 };
 
 struct dpll_data {
-	u16			mult_div1_reg;
+	void __iomem		*mult_div1_reg;
 	u32			mult_mask;
 	u32			div1_mask;
 	u16			last_rounded_m;
@@ -41,18 +41,18 @@ struct dpll_data {
 	u16			max_multiplier;
 	u8			max_divider;
 	u32			max_tolerance;
-	u16			idlest_reg;
+	void __iomem		*idlest_reg;
 	u32			idlest_mask;
 	struct clk		*bypass_clk;
 #  if defined(CONFIG_ARCH_OMAP3)
 	u32			freqsel_mask;
 	u8			modes;
-	u16			control_reg;
+	void __iomem		*control_reg;
 	u32			enable_mask;
 	u8			auto_recal_bit;
 	u8			recal_en_bit;
 	u8			recal_st_bit;
-	u16			autoidle_reg;
+	void __iomem		*autoidle_reg;
 	u32			autoidle_mask;
 #  endif
 };
@@ -67,7 +67,7 @@ struct clk {
 	struct clk		*parent;
 	unsigned long		rate;
 	__u32			flags;
-	u16			enable_reg;
+	void __iomem		*enable_reg;
 	__u8			enable_bit;
 	__s8			usecount;
 	u8			idlest_bit;
@@ -79,7 +79,7 @@ struct clk {
 	void			(*disable)(struct clk *);
 #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
 	u8			fixed_div;
-	u16			clksel_reg;
+	void __iomem		*clksel_reg;
 	u32			clksel_mask;
 	const struct clksel	*clksel;
 	struct dpll_data	*dpll_data;

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  8:00       ` Tony Lindgren
@ 2008-09-24  8:03         ` Paul Walmsley
  2008-09-24  8:06           ` Tony Lindgren
  0 siblings, 1 reply; 16+ messages in thread
From: Paul Walmsley @ 2008-09-24  8:03 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Arun KS, Hiroshi DOYU, linux-omap

On Wed, 24 Sep 2008, Tony Lindgren wrote:

> * Tony Lindgren <tony@atomide.com> [080924 10:12]:
> > * Arun KS <arunks@mistralsolutions.com> [080924 10:05]:
> > > On Wed, Sep 24, 2008 at 9:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > > > Hi,
> > > >
> > > > How about introducing a new typedef in order to accommodate the
> > > > different size of enable_regs without ifdefs?
> > > >
> > > > I think that this is apparently the case categorized into (a) in
> > > > 'CodyingStyle'.
> > > 
> > > I am not sure which is the right way to do. Is this what you meant?
> > 
> > No, let's not do this. It should be void __iomem * for sure. Most of
> > the omap1 clock registers looks just fine for void __iomem *, let's
> > figure out which registers are broken.
> 
> I've pushed a fix for this. Turns out I missed some conversions to
> void __iomem * while merging code from arm-devel branch. Fix also
> attached.

Those registers should be u16 for OMAP2/3.  OMAP1 needs something similar, 
which will also get rid of those casts to void __iomem * - hacking on 
something now ...
.

- Paul

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  8:03         ` Paul Walmsley
@ 2008-09-24  8:06           ` Tony Lindgren
  2008-09-24  8:08             ` Paul Walmsley
  0 siblings, 1 reply; 16+ messages in thread
From: Tony Lindgren @ 2008-09-24  8:06 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: Arun KS, Hiroshi DOYU, linux-omap

* Paul Walmsley <paul@booyaka.com> [080924 11:03]:
> On Wed, 24 Sep 2008, Tony Lindgren wrote:
> 
> > * Tony Lindgren <tony@atomide.com> [080924 10:12]:
> > > * Arun KS <arunks@mistralsolutions.com> [080924 10:05]:
> > > > On Wed, Sep 24, 2008 at 9:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > > > > Hi,
> > > > >
> > > > > How about introducing a new typedef in order to accommodate the
> > > > > different size of enable_regs without ifdefs?
> > > > >
> > > > > I think that this is apparently the case categorized into (a) in
> > > > > 'CodyingStyle'.
> > > > 
> > > > I am not sure which is the right way to do. Is this what you meant?
> > > 
> > > No, let's not do this. It should be void __iomem * for sure. Most of
> > > the omap1 clock registers looks just fine for void __iomem *, let's
> > > figure out which registers are broken.
> > 
> > I've pushed a fix for this. Turns out I missed some conversions to
> > void __iomem * while merging code from arm-devel branch. Fix also
> > attached.
> 
> Those registers should be u16 for OMAP2/3.  OMAP1 needs something similar, 
> which will also get rid of those casts to void __iomem * - hacking on 
> something now ...

Argh, sorry. Yeah it's an offset for omap2/3.. Will revert immediately.

Tony

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  8:06           ` Tony Lindgren
@ 2008-09-24  8:08             ` Paul Walmsley
  2008-09-24  8:34               ` Tony Lindgren
  0 siblings, 1 reply; 16+ messages in thread
From: Paul Walmsley @ 2008-09-24  8:08 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Arun KS, Hiroshi DOYU, linux-omap

On Wed, 24 Sep 2008, Tony Lindgren wrote:

> * Paul Walmsley <paul@booyaka.com> [080924 11:03]:
> > On Wed, 24 Sep 2008, Tony Lindgren wrote:
> > 
> > > * Tony Lindgren <tony@atomide.com> [080924 10:12]:
> > > > * Arun KS <arunks@mistralsolutions.com> [080924 10:05]:
> > > > > On Wed, Sep 24, 2008 at 9:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > > > > > Hi,
> > > > > >
> > > > > > How about introducing a new typedef in order to accommodate the
> > > > > > different size of enable_regs without ifdefs?
> > > > > >
> > > > > > I think that this is apparently the case categorized into (a) in
> > > > > > 'CodyingStyle'.
> > > > > 
> > > > > I am not sure which is the right way to do. Is this what you meant?
> > > > 
> > > > No, let's not do this. It should be void __iomem * for sure. Most of
> > > > the omap1 clock registers looks just fine for void __iomem *, let's
> > > > figure out which registers are broken.
> > > 
> > > I've pushed a fix for this. Turns out I missed some conversions to
> > > void __iomem * while merging code from arm-devel branch. Fix also
> > > attached.
> > 
> > Those registers should be u16 for OMAP2/3.  OMAP1 needs something similar, 
> > which will also get rid of those casts to void __iomem * - hacking on 
> > something now ...
> 
> Argh, sorry. Yeah it's an offset for omap2/3.. Will revert immediately.

No problem, it's really my fault - I need to start testing on OMAP1 also.

- Paul

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  8:08             ` Paul Walmsley
@ 2008-09-24  8:34               ` Tony Lindgren
  2008-09-24  9:03                 ` Arun KS
  2008-09-24 14:06                 ` Paul Walmsley
  0 siblings, 2 replies; 16+ messages in thread
From: Tony Lindgren @ 2008-09-24  8:34 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: Arun KS, Hiroshi DOYU, linux-omap

[-- Attachment #1: Type: text/plain, Size: 1664 bytes --]

* Paul Walmsley <paul@booyaka.com> [080924 11:08]:
> On Wed, 24 Sep 2008, Tony Lindgren wrote:
> 
> > * Paul Walmsley <paul@booyaka.com> [080924 11:03]:
> > > On Wed, 24 Sep 2008, Tony Lindgren wrote:
> > > 
> > > > * Tony Lindgren <tony@atomide.com> [080924 10:12]:
> > > > > * Arun KS <arunks@mistralsolutions.com> [080924 10:05]:
> > > > > > On Wed, Sep 24, 2008 at 9:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > How about introducing a new typedef in order to accommodate the
> > > > > > > different size of enable_regs without ifdefs?
> > > > > > >
> > > > > > > I think that this is apparently the case categorized into (a) in
> > > > > > > 'CodyingStyle'.
> > > > > > 
> > > > > > I am not sure which is the right way to do. Is this what you meant?
> > > > > 
> > > > > No, let's not do this. It should be void __iomem * for sure. Most of
> > > > > the omap1 clock registers looks just fine for void __iomem *, let's
> > > > > figure out which registers are broken.
> > > > 
> > > > I've pushed a fix for this. Turns out I missed some conversions to
> > > > void __iomem * while merging code from arm-devel branch. Fix also
> > > > attached.
> > > 
> > > Those registers should be u16 for OMAP2/3.  OMAP1 needs something similar, 
> > > which will also get rid of those casts to void __iomem * - hacking on 
> > > something now ...
> > 
> > Argh, sorry. Yeah it's an offset for omap2/3.. Will revert immediately.
> 
> No problem, it's really my fault - I need to start testing on OMAP1 also.

How about this for a quick fix? It produces now a bunch of warnings
until omap1 clock uses offsets too.

Tony


[-- Attachment #2: omap1-clock-fix-v2.patch --]
[-- Type: text/x-diff, Size: 18134 bytes --]

>From fff89ca6ab98a359ff114da7db3803b700258319 Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Wed, 24 Sep 2008 11:31:03 +0300
Subject: [PATCH] Fix omap1 clock issues

This fixes booting, and is a step toward fixing things properly:

- Make enable_reg u32 instead of u16
- Get rid of VIRTUAL_IO_ADDRESS for clocks
- Use __raw_read/write instead of omap_read/write for clock registers

This patch adds a bunch of compile warnings until omap1 clock
also uses offsets.

Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/mach-omap1/clock.c b/arch/arm/mach-omap1/clock.c
index 90ae0ef..799867c 100644
--- a/arch/arm/mach-omap1/clock.c
+++ b/arch/arm/mach-omap1/clock.c
@@ -41,7 +41,7 @@ static void omap1_watchdog_recalc(struct clk * clk)
 
 static void omap1_uart_recalc(struct clk * clk)
 {
-	unsigned int val = omap_readl(clk->enable_reg);
+	unsigned int val = __raw_readl(clk->enable_reg);
 	if (val & clk->enable_bit)
 		clk->rate = 48000000;
 	else
@@ -372,14 +372,14 @@ static int omap1_set_uart_rate(struct clk * clk, unsigned long rate)
 {
 	unsigned int val;
 
-	val = omap_readl(clk->enable_reg);
+	val = __raw_readl(clk->enable_reg);
 	if (rate == 12000000)
 		val &= ~(1 << clk->enable_bit);
 	else if (rate == 48000000)
 		val |= (1 << clk->enable_bit);
 	else
 		return -EINVAL;
-	omap_writel(val, clk->enable_reg);
+	__raw_writel(val, clk->enable_reg);
 	clk->rate = rate;
 
 	return 0;
@@ -398,8 +398,8 @@ static int omap1_set_ext_clk_rate(struct clk * clk, unsigned long rate)
 	else
 		ratio_bits = (dsor - 2) << 2;
 
-	ratio_bits |= omap_readw(clk->enable_reg) & ~0xfd;
-	omap_writew(ratio_bits, clk->enable_reg);
+	ratio_bits |= __raw_readw(clk->enable_reg) & ~0xfd;
+	__raw_writew(ratio_bits, clk->enable_reg);
 
 	return 0;
 }
@@ -440,8 +440,8 @@ static void omap1_init_ext_clk(struct clk * clk)
 	__u16 ratio_bits;
 
 	/* Determine current rate and ensure clock is based on 96MHz APLL */
-	ratio_bits = omap_readw(clk->enable_reg) & ~1;
-	omap_writew(ratio_bits, clk->enable_reg);
+	ratio_bits = __raw_readw(clk->enable_reg) & ~1;
+	__raw_writew(ratio_bits, clk->enable_reg);
 
 	ratio_bits = (ratio_bits & 0xfc) >> 2;
 	if (ratio_bits > 6)
@@ -506,25 +506,13 @@ static int omap1_clk_enable_generic(struct clk *clk)
 	}
 
 	if (clk->flags & ENABLE_REG_32BIT) {
-		if (clk->flags & VIRTUAL_IO_ADDRESS) {
-			regval32 = __raw_readl(clk->enable_reg);
-			regval32 |= (1 << clk->enable_bit);
-			__raw_writel(regval32, clk->enable_reg);
-		} else {
-			regval32 = omap_readl(clk->enable_reg);
-			regval32 |= (1 << clk->enable_bit);
-			omap_writel(regval32, clk->enable_reg);
-		}
+		regval32 = __raw_readl(clk->enable_reg);
+		regval32 |= (1 << clk->enable_bit);
+		__raw_writel(regval32, clk->enable_reg);
 	} else {
-		if (clk->flags & VIRTUAL_IO_ADDRESS) {
-			regval16 = __raw_readw(clk->enable_reg);
-			regval16 |= (1 << clk->enable_bit);
-			__raw_writew(regval16, clk->enable_reg);
-		} else {
-			regval16 = omap_readw(clk->enable_reg);
-			regval16 |= (1 << clk->enable_bit);
-			omap_writew(regval16, clk->enable_reg);
-		}
+		regval16 = __raw_readw(clk->enable_reg);
+		regval16 |= (1 << clk->enable_bit);
+		__raw_writew(regval16, clk->enable_reg);
 	}
 
 	return 0;
@@ -539,25 +527,13 @@ static void omap1_clk_disable_generic(struct clk *clk)
 		return;
 
 	if (clk->flags & ENABLE_REG_32BIT) {
-		if (clk->flags & VIRTUAL_IO_ADDRESS) {
-			regval32 = __raw_readl(clk->enable_reg);
-			regval32 &= ~(1 << clk->enable_bit);
-			__raw_writel(regval32, clk->enable_reg);
-		} else {
-			regval32 = omap_readl(clk->enable_reg);
-			regval32 &= ~(1 << clk->enable_bit);
-			omap_writel(regval32, clk->enable_reg);
-		}
+		regval32 = __raw_readl(clk->enable_reg);
+		regval32 &= ~(1 << clk->enable_bit);
+		__raw_writel(regval32, clk->enable_reg);
 	} else {
-		if (clk->flags & VIRTUAL_IO_ADDRESS) {
-			regval16 = __raw_readw(clk->enable_reg);
-			regval16 &= ~(1 << clk->enable_bit);
-			__raw_writew(regval16, clk->enable_reg);
-		} else {
-			regval16 = omap_readw(clk->enable_reg);
-			regval16 &= ~(1 << clk->enable_bit);
-			omap_writew(regval16, clk->enable_reg);
-		}
+		regval16 = __raw_readw(clk->enable_reg);
+		regval16 &= ~(1 << clk->enable_bit);
+		__raw_writew(regval16, clk->enable_reg);
 	}
 }
 
@@ -632,17 +608,10 @@ static void __init omap1_clk_disable_unused(struct clk *clk)
 	}
 
 	/* Is the clock already disabled? */
-	if (clk->flags & ENABLE_REG_32BIT) {
-		if (clk->flags & VIRTUAL_IO_ADDRESS)
-			regval32 = __raw_readl(clk->enable_reg);
-			else
-				regval32 = omap_readl(clk->enable_reg);
-	} else {
-		if (clk->flags & VIRTUAL_IO_ADDRESS)
-			regval32 = __raw_readw(clk->enable_reg);
-		else
-			regval32 = omap_readw(clk->enable_reg);
-	}
+	if (clk->flags & ENABLE_REG_32BIT)
+		regval32 = __raw_readl(clk->enable_reg);
+	else
+		regval32 = __raw_readw(clk->enable_reg);
 
 	if ((regval32 & (1 << clk->enable_bit)) == 0)
 		return;
diff --git a/arch/arm/mach-omap1/clock.h b/arch/arm/mach-omap1/clock.h
index 5635b51..6d4f77d 100644
--- a/arch/arm/mach-omap1/clock.h
+++ b/arch/arm/mach-omap1/clock.h
@@ -174,7 +174,7 @@ static struct arm_idlect1_clk ck_dpll1out = {
 		.parent		= &ck_dpll1,
 		.flags		= CLOCK_IN_OMAP16XX | CLOCK_IDLE_CONTROL |
 				  ENABLE_REG_32BIT | RATE_PROPAGATES,
-		.enable_reg	= (void __iomem *)ARM_IDLECT2,
+		.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 		.enable_bit	= EN_CKOUT_ARM,
 		.recalc		= &followparent_recalc,
 		.enable		= &omap1_clk_enable_generic,
@@ -188,7 +188,7 @@ static struct clk sossi_ck = {
 	.parent		= &ck_dpll1out.clk,
 	.flags		= CLOCK_IN_OMAP16XX | CLOCK_NO_IDLE_PARENT |
 			  ENABLE_REG_32BIT,
-	.enable_reg	= (void __iomem *)MOD_CONF_CTRL_1,
+	.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_1),
 	.enable_bit	= 16,
 	.recalc		= &omap1_sossi_recalc,
 	.set_rate	= &omap1_set_sossi_rate,
@@ -215,7 +215,7 @@ static struct arm_idlect1_clk armper_ck = {
 		.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 				  CLOCK_IN_OMAP310 | RATE_CKCTL |
 				  CLOCK_IDLE_CONTROL,
-		.enable_reg	= (void __iomem *)ARM_IDLECT2,
+		.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 		.enable_bit	= EN_PERCK,
 		.rate_offset	= CKCTL_PERDIV_OFFSET,
 		.recalc		= &omap1_ckctl_recalc,
@@ -229,7 +229,7 @@ static struct clk arm_gpio_ck = {
 	.name		= "arm_gpio_ck",
 	.parent		= &ck_dpll1,
 	.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP310,
-	.enable_reg	= (void __iomem *)ARM_IDLECT2,
+	.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 	.enable_bit	= EN_GPIOCK,
 	.recalc		= &followparent_recalc,
 	.enable		= &omap1_clk_enable_generic,
@@ -242,7 +242,7 @@ static struct arm_idlect1_clk armxor_ck = {
 		.parent		= &ck_ref,
 		.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 				  CLOCK_IN_OMAP310 | CLOCK_IDLE_CONTROL,
-		.enable_reg	= (void __iomem *)ARM_IDLECT2,
+		.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 		.enable_bit	= EN_XORPCK,
 		.recalc		= &followparent_recalc,
 		.enable		= &omap1_clk_enable_generic,
@@ -257,7 +257,7 @@ static struct arm_idlect1_clk armtim_ck = {
 		.parent		= &ck_ref,
 		.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 				  CLOCK_IN_OMAP310 | CLOCK_IDLE_CONTROL,
-		.enable_reg	= (void __iomem *)ARM_IDLECT2,
+		.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 		.enable_bit	= EN_TIMCK,
 		.recalc		= &followparent_recalc,
 		.enable		= &omap1_clk_enable_generic,
@@ -272,7 +272,7 @@ static struct arm_idlect1_clk armwdt_ck = {
 		.parent		= &ck_ref,
 		.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 				  CLOCK_IN_OMAP310 | CLOCK_IDLE_CONTROL,
-		.enable_reg	= (void __iomem *)ARM_IDLECT2,
+		.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 		.enable_bit	= EN_WDTCK,
 		.recalc		= &omap1_watchdog_recalc,
 		.enable		= &omap1_clk_enable_generic,
@@ -300,7 +300,7 @@ static struct clk dsp_ck = {
 	.parent		= &ck_dpll1,
 	.flags		= CLOCK_IN_OMAP310 | CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 			  RATE_CKCTL,
-	.enable_reg	= (void __iomem *)ARM_CKCTL,
+	.enable_reg	= OMAP1_IO_ADDRESS(ARM_CKCTL),
 	.enable_bit	= EN_DSPCK,
 	.rate_offset	= CKCTL_DSPDIV_OFFSET,
 	.recalc		= &omap1_ckctl_recalc,
@@ -323,7 +323,7 @@ static struct clk dspper_ck = {
 	.name		= "dspper_ck",
 	.parent		= &ck_dpll1,
 	.flags		= CLOCK_IN_OMAP310 | CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
-			  RATE_CKCTL | VIRTUAL_IO_ADDRESS,
+			  RATE_CKCTL,
 	.enable_reg	= DSP_IDLECT2,
 	.enable_bit	= EN_PERCK,
 	.rate_offset	= CKCTL_PERDIV_OFFSET,
@@ -336,8 +336,7 @@ static struct clk dspper_ck = {
 static struct clk dspxor_ck = {
 	.name		= "dspxor_ck",
 	.parent		= &ck_ref,
-	.flags		= CLOCK_IN_OMAP310 | CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
-			  VIRTUAL_IO_ADDRESS,
+	.flags		= CLOCK_IN_OMAP310 | CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX,
 	.enable_reg	= DSP_IDLECT2,
 	.enable_bit	= EN_XORPCK,
 	.recalc		= &followparent_recalc,
@@ -348,8 +347,7 @@ static struct clk dspxor_ck = {
 static struct clk dsptim_ck = {
 	.name		= "dsptim_ck",
 	.parent		= &ck_ref,
-	.flags		= CLOCK_IN_OMAP310 | CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
-			  VIRTUAL_IO_ADDRESS,
+	.flags		= CLOCK_IN_OMAP310 | CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX,
 	.enable_reg	= DSP_IDLECT2,
 	.enable_bit	= EN_DSPTIMCK,
 	.recalc		= &followparent_recalc,
@@ -404,7 +402,7 @@ static struct clk l3_ocpi_ck = {
 	.name		= "l3_ocpi_ck",
 	.parent		= &tc_ck.clk,
 	.flags		= CLOCK_IN_OMAP16XX,
-	.enable_reg	= (void __iomem *)ARM_IDLECT3,
+	.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT3),
 	.enable_bit	= EN_OCPI_CK,
 	.recalc		= &followparent_recalc,
 	.enable		= &omap1_clk_enable_generic,
@@ -415,7 +413,7 @@ static struct clk tc1_ck = {
 	.name		= "tc1_ck",
 	.parent		= &tc_ck.clk,
 	.flags		= CLOCK_IN_OMAP16XX,
-	.enable_reg	= (void __iomem *)ARM_IDLECT3,
+	.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT3),
 	.enable_bit	= EN_TC1_CK,
 	.recalc		= &followparent_recalc,
 	.enable		= &omap1_clk_enable_generic,
@@ -426,7 +424,7 @@ static struct clk tc2_ck = {
 	.name		= "tc2_ck",
 	.parent		= &tc_ck.clk,
 	.flags		= CLOCK_IN_OMAP16XX,
-	.enable_reg	= (void __iomem *)ARM_IDLECT3,
+	.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT3),
 	.enable_bit	= EN_TC2_CK,
 	.recalc		= &followparent_recalc,
 	.enable		= &omap1_clk_enable_generic,
@@ -459,7 +457,7 @@ static struct arm_idlect1_clk api_ck = {
 		.parent		= &tc_ck.clk,
 		.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 				  CLOCK_IN_OMAP310 | CLOCK_IDLE_CONTROL,
-		.enable_reg	= (void __iomem *)ARM_IDLECT2,
+		.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 		.enable_bit	= EN_APICK,
 		.recalc		= &followparent_recalc,
 		.enable		= &omap1_clk_enable_generic,
@@ -474,7 +472,7 @@ static struct arm_idlect1_clk lb_ck = {
 		.parent		= &tc_ck.clk,
 		.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP310 |
 				  CLOCK_IDLE_CONTROL,
-		.enable_reg	= (void __iomem *)ARM_IDLECT2,
+		.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 		.enable_bit	= EN_LBCK,
 		.recalc		= &followparent_recalc,
 		.enable		= &omap1_clk_enable_generic,
@@ -505,7 +503,7 @@ static struct clk lcd_ck_16xx = {
 	.name		= "lcd_ck",
 	.parent		= &ck_dpll1,
 	.flags		= CLOCK_IN_OMAP16XX | CLOCK_IN_OMAP730 | RATE_CKCTL,
-	.enable_reg	= (void __iomem *)ARM_IDLECT2,
+	.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 	.enable_bit	= EN_LCDCK,
 	.rate_offset	= CKCTL_LCDDIV_OFFSET,
 	.recalc		= &omap1_ckctl_recalc,
@@ -519,7 +517,7 @@ static struct arm_idlect1_clk lcd_ck_1510 = {
 		.parent		= &ck_dpll1,
 		.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP310 |
 				  RATE_CKCTL | CLOCK_IDLE_CONTROL,
-		.enable_reg	= (void __iomem *)ARM_IDLECT2,
+		.enable_reg	= OMAP1_IO_ADDRESS(ARM_IDLECT2),
 		.enable_bit	= EN_LCDCK,
 		.rate_offset	= CKCTL_LCDDIV_OFFSET,
 		.recalc		= &omap1_ckctl_recalc,
@@ -537,7 +535,7 @@ static struct clk uart1_1510 = {
 	.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP310 |
 			  ENABLE_REG_32BIT | ALWAYS_ENABLED |
 			  CLOCK_NO_IDLE_PARENT,
-	.enable_reg	= (void __iomem *)MOD_CONF_CTRL_0,
+	.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_0),
 	.enable_bit	= 29,	/* Chooses between 12MHz and 48MHz */
 	.set_rate	= &omap1_set_uart_rate,
 	.recalc		= &omap1_uart_recalc,
@@ -553,7 +551,7 @@ static struct uart_clk uart1_16xx = {
 		.rate		= 48000000,
 		.flags		= CLOCK_IN_OMAP16XX | RATE_FIXED |
 				  ENABLE_REG_32BIT | CLOCK_NO_IDLE_PARENT,
-		.enable_reg	= (void __iomem *)MOD_CONF_CTRL_0,
+		.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_0),
 		.enable_bit	= 29,
 		.enable		= &omap1_clk_enable_uart_functional,
 		.disable	= &omap1_clk_disable_uart_functional,
@@ -569,7 +567,7 @@ static struct clk uart2_ck = {
 	.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 			  CLOCK_IN_OMAP310 | ENABLE_REG_32BIT |
 			  ALWAYS_ENABLED | CLOCK_NO_IDLE_PARENT,
-	.enable_reg	= (void __iomem *)MOD_CONF_CTRL_0,
+	.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_0),
 	.enable_bit	= 30,	/* Chooses between 12MHz and 48MHz */
 	.set_rate	= &omap1_set_uart_rate,
 	.recalc		= &omap1_uart_recalc,
@@ -585,7 +583,7 @@ static struct clk uart3_1510 = {
 	.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP310 |
 			  ENABLE_REG_32BIT | ALWAYS_ENABLED |
 			  CLOCK_NO_IDLE_PARENT,
-	.enable_reg	= (void __iomem *)MOD_CONF_CTRL_0,
+	.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_0),
 	.enable_bit	= 31,	/* Chooses between 12MHz and 48MHz */
 	.set_rate	= &omap1_set_uart_rate,
 	.recalc		= &omap1_uart_recalc,
@@ -601,7 +599,7 @@ static struct uart_clk uart3_16xx = {
 		.rate		= 48000000,
 		.flags		= CLOCK_IN_OMAP16XX | RATE_FIXED |
 				  ENABLE_REG_32BIT | CLOCK_NO_IDLE_PARENT,
-		.enable_reg	= (void __iomem *)MOD_CONF_CTRL_0,
+		.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_0),
 		.enable_bit	= 31,
 		.enable		= &omap1_clk_enable_uart_functional,
 		.disable	= &omap1_clk_disable_uart_functional,
@@ -615,7 +613,7 @@ static struct clk usb_clko = {	/* 6 MHz output on W4_USB_CLKO */
 	.rate		= 6000000,
 	.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 			  CLOCK_IN_OMAP310 | RATE_FIXED | ENABLE_REG_32BIT,
-	.enable_reg	= (void __iomem *)ULPD_CLOCK_CTRL,
+	.enable_reg	= OMAP1_IO_ADDRESS(ULPD_CLOCK_CTRL),
 	.enable_bit	= USB_MCLK_EN_BIT,
 	.enable		= &omap1_clk_enable_generic,
 	.disable	= &omap1_clk_disable_generic,
@@ -627,7 +625,7 @@ static struct clk usb_hhc_ck1510 = {
 	.rate		= 48000000, /* Actually 2 clocks, 12MHz and 48MHz */
 	.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP310 |
 			  RATE_FIXED | ENABLE_REG_32BIT,
-	.enable_reg	= (void __iomem *)MOD_CONF_CTRL_0,
+	.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_0),
 	.enable_bit	= USB_HOST_HHC_UHOST_EN,
 	.enable		= &omap1_clk_enable_generic,
 	.disable	= &omap1_clk_disable_generic,
@@ -640,7 +638,7 @@ static struct clk usb_hhc_ck16xx = {
 	/* OTG_SYSCON_2.OTG_PADEN == 0 (not 1510-compatible) */
 	.flags		= CLOCK_IN_OMAP16XX |
 			  RATE_FIXED | ENABLE_REG_32BIT,
-	.enable_reg	= (void __iomem *)OTG_BASE + 0x08 /* OTG_SYSCON_2 */,
+	.enable_reg	= OMAP1_IO_ADDRESS(OTG_BASE + 0x08), /* OTG_SYSCON_2 */
 	.enable_bit	= 8 /* UHOST_EN */,
 	.enable		= &omap1_clk_enable_generic,
 	.disable	= &omap1_clk_disable_generic,
@@ -651,7 +649,7 @@ static struct clk usb_dc_ck = {
 	/* Direct from ULPD, no parent */
 	.rate		= 48000000,
 	.flags		= CLOCK_IN_OMAP16XX | RATE_FIXED,
-	.enable_reg	= (void __iomem *)SOFT_REQ_REG,
+	.enable_reg	= OMAP1_IO_ADDRESS(SOFT_REQ_REG),
 	.enable_bit	= 4,
 	.enable		= &omap1_clk_enable_generic,
 	.disable	= &omap1_clk_disable_generic,
@@ -662,7 +660,7 @@ static struct clk mclk_1510 = {
 	/* Direct from ULPD, no parent. May be enabled by ext hardware. */
 	.rate		= 12000000,
  	.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP310 | RATE_FIXED,
- 	.enable_reg	= (void __iomem *)SOFT_REQ_REG,
+ 	.enable_reg	= OMAP1_IO_ADDRESS(SOFT_REQ_REG),
  	.enable_bit	= 6,
 	.enable		= &omap1_clk_enable_generic,
 	.disable	= &omap1_clk_disable_generic,
@@ -672,7 +670,7 @@ static struct clk mclk_16xx = {
 	.name		= "mclk",
 	/* Direct from ULPD, no parent. May be enabled by ext hardware. */
 	.flags		= CLOCK_IN_OMAP16XX,
-	.enable_reg	= (void __iomem *)COM_CLK_DIV_CTRL_SEL,
+	.enable_reg	= OMAP1_IO_ADDRESS(COM_CLK_DIV_CTRL_SEL),
 	.enable_bit	= COM_ULPD_PLL_CLK_REQ,
 	.set_rate	= &omap1_set_ext_clk_rate,
 	.round_rate	= &omap1_round_ext_clk_rate,
@@ -694,7 +692,7 @@ static struct clk bclk_16xx = {
 	.name		= "bclk",
 	/* Direct from ULPD, no parent. May be enabled by ext hardware. */
 	.flags		= CLOCK_IN_OMAP16XX,
-	.enable_reg	= (void __iomem *)SWD_CLK_DIV_CTRL_SEL,
+	.enable_reg	= OMAP1_IO_ADDRESS(SWD_CLK_DIV_CTRL_SEL),
 	.enable_bit	= SWD_ULPD_PLL_CLK_REQ,
 	.set_rate	= &omap1_set_ext_clk_rate,
 	.round_rate	= &omap1_round_ext_clk_rate,
@@ -712,7 +710,7 @@ static struct clk mmc1_ck = {
 	.flags		= CLOCK_IN_OMAP1510 | CLOCK_IN_OMAP16XX |
 			  CLOCK_IN_OMAP310 | RATE_FIXED | ENABLE_REG_32BIT |
 			  CLOCK_NO_IDLE_PARENT,
-	.enable_reg	= (void __iomem *)MOD_CONF_CTRL_0,
+	.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_0),
 	.enable_bit	= 23,
 	.enable		= &omap1_clk_enable_generic,
 	.disable	= &omap1_clk_disable_generic,
@@ -726,7 +724,7 @@ static struct clk mmc2_ck = {
 	.rate		= 48000000,
 	.flags		= CLOCK_IN_OMAP16XX |
 			  RATE_FIXED | ENABLE_REG_32BIT | CLOCK_NO_IDLE_PARENT,
-	.enable_reg	= (void __iomem *)MOD_CONF_CTRL_0,
+	.enable_reg	= OMAP1_IO_ADDRESS(MOD_CONF_CTRL_0),
 	.enable_bit	= 20,
 	.enable		= &omap1_clk_enable_generic,
 	.disable	= &omap1_clk_disable_generic,
diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index f26ad8c..9088925 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -67,7 +67,7 @@ struct clk {
 	struct clk		*parent;
 	unsigned long		rate;
 	__u32			flags;
-	u16			enable_reg;
+	u32			enable_reg;
 	__u8			enable_bit;
 	__s8			usecount;
 	u8			idlest_bit;
@@ -137,7 +137,7 @@ extern void clk_init_cpufreq_table(struct cpufreq_frequency_table **table);
 #define VIRTUAL_CLOCK		(1 << 3)	/* Composite clock from table */
 #define ALWAYS_ENABLED		(1 << 4)	/* Clock cannot be disabled */
 #define ENABLE_REG_32BIT	(1 << 5)	/* Use 32-bit access */
-#define VIRTUAL_IO_ADDRESS	(1 << 6)	/* Clock in virtual address */
+
 #define CLOCK_IDLE_CONTROL	(1 << 7)
 #define CLOCK_NO_IDLE_PARENT	(1 << 8)
 #define DELAYED_APP		(1 << 9)	/* Delay application of clock */

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  8:34               ` Tony Lindgren
@ 2008-09-24  9:03                 ` Arun KS
  2008-09-24 14:06                 ` Paul Walmsley
  1 sibling, 0 replies; 16+ messages in thread
From: Arun KS @ 2008-09-24  9:03 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Paul Walmsley, Hiroshi DOYU, linux-omap

On Wed, Sep 24, 2008 at 2:04 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Paul Walmsley <paul@booyaka.com> [080924 11:08]:
>> On Wed, 24 Sep 2008, Tony Lindgren wrote:
>>
>> > * Paul Walmsley <paul@booyaka.com> [080924 11:03]:
>> > > On Wed, 24 Sep 2008, Tony Lindgren wrote:
>> > >
>> > > > * Tony Lindgren <tony@atomide.com> [080924 10:12]:
>> > > > > * Arun KS <arunks@mistralsolutions.com> [080924 10:05]:
>> > > > > > On Wed, Sep 24, 2008 at 9:21 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> > > > > > > Hi,
>> > > > > > >
>> > > > > > > How about introducing a new typedef in order to accommodate the
>> > > > > > > different size of enable_regs without ifdefs?
>> > > > > > >
>> > > > > > > I think that this is apparently the case categorized into (a) in
>> > > > > > > 'CodyingStyle'.
>> > > > > >
>> > > > > > I am not sure which is the right way to do. Is this what you meant?
>> > > > >
>> > > > > No, let's not do this. It should be void __iomem * for sure. Most of
>> > > > > the omap1 clock registers looks just fine for void __iomem *, let's
>> > > > > figure out which registers are broken.
>> > > >
>> > > > I've pushed a fix for this. Turns out I missed some conversions to
>> > > > void __iomem * while merging code from arm-devel branch. Fix also
>> > > > attached.
>> > >
>> > > Those registers should be u16 for OMAP2/3.  OMAP1 needs something similar,
>> > > which will also get rid of those casts to void __iomem * - hacking on
>> > > something now ...
>> >
>> > Argh, sorry. Yeah it's an offset for omap2/3.. Will revert immediately.
>>
>> No problem, it's really my fault - I need to start testing on OMAP1 also.
>
> How about this for a quick fix? It produces now a bunch of warnings
> until omap1 clock uses offsets too.

Tested, it works.

Its good to have this fix till omap1 clock uses offsets.

Arun

>
> Tony
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24  8:34               ` Tony Lindgren
  2008-09-24  9:03                 ` Arun KS
@ 2008-09-24 14:06                 ` Paul Walmsley
  2008-09-24 14:27                   ` Tony Lindgren
  1 sibling, 1 reply; 16+ messages in thread
From: Paul Walmsley @ 2008-09-24 14:06 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Arun KS, Hiroshi DOYU, linux-omap

On Wed, 24 Sep 2008, Tony Lindgren wrote:

> How about this for a quick fix? It produces now a bunch of warnings
> until omap1 clock uses offsets too.

Looks good to me.  Good to see that VIRTUAL_IO_ADDRESS flag gone -- 


- Paul

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24 14:06                 ` Paul Walmsley
@ 2008-09-24 14:27                   ` Tony Lindgren
  2008-09-24 14:35                     ` Paul Walmsley
  0 siblings, 1 reply; 16+ messages in thread
From: Tony Lindgren @ 2008-09-24 14:27 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: Arun KS, Hiroshi DOYU, linux-omap

* Paul Walmsley <paul@booyaka.com> [080924 17:06]:
> On Wed, 24 Sep 2008, Tony Lindgren wrote:
> 
> > How about this for a quick fix? It produces now a bunch of warnings
> > until omap1 clock uses offsets too.
> 
> Looks good to me.  Good to see that VIRTUAL_IO_ADDRESS flag gone -- 

OK, pushed with a minror change. I've marked DSP addresses as IOMEM.

Tony

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24 14:27                   ` Tony Lindgren
@ 2008-09-24 14:35                     ` Paul Walmsley
  2008-09-24 14:43                       ` Tony Lindgren
  0 siblings, 1 reply; 16+ messages in thread
From: Paul Walmsley @ 2008-09-24 14:35 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Arun KS, Hiroshi DOYU, linux-omap

On Wed, 24 Sep 2008, Tony Lindgren wrote:

> * Paul Walmsley <paul@booyaka.com> [080924 17:06]:
> > On Wed, 24 Sep 2008, Tony Lindgren wrote:
> > 
> > > How about this for a quick fix? It produces now a bunch of warnings
> > > until omap1 clock uses offsets too.
> > 
> > Looks good to me.  Good to see that VIRTUAL_IO_ADDRESS flag gone -- 
> 
> OK, pushed with a minror change. I've marked DSP addresses as IOMEM.

Are those marked as IOMEM() already in 
arch/arm/plat-omap/include/mach/hardware.h ?


- Paul

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Temporary fix to boot OMAP1 based boards
  2008-09-24 14:35                     ` Paul Walmsley
@ 2008-09-24 14:43                       ` Tony Lindgren
  0 siblings, 0 replies; 16+ messages in thread
From: Tony Lindgren @ 2008-09-24 14:43 UTC (permalink / raw)
  To: Paul Walmsley; +Cc: Arun KS, Hiroshi DOYU, linux-omap

* Paul Walmsley <paul@booyaka.com> [080924 17:35]:
> On Wed, 24 Sep 2008, Tony Lindgren wrote:
> 
> > * Paul Walmsley <paul@booyaka.com> [080924 17:06]:
> > > On Wed, 24 Sep 2008, Tony Lindgren wrote:
> > > 
> > > > How about this for a quick fix? It produces now a bunch of warnings
> > > > until omap1 clock uses offsets too.
> > > 
> > > Looks good to me.  Good to see that VIRTUAL_IO_ADDRESS flag gone -- 
> > 
> > OK, pushed with a minror change. I've marked DSP addresses as IOMEM.
> 
> Are those marked as IOMEM() already in 
> arch/arm/plat-omap/include/mach/hardware.h ?

No, but they could since it does not change the offset. Marking anything
OMAP1_IO_ADDRESS would require grepping for that register naturally..

Tony

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2008-09-24 14:43 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-23 15:23 [PATCH] Temporary fix to boot OMAP1 based boards Arun KS
2008-09-23 22:43 ` Felipe Balbi
2008-09-24  4:50   ` Arun KS
2008-09-24  3:51 ` Hiroshi DOYU
2008-09-24  7:05   ` Arun KS
2008-09-24  7:12     ` Tony Lindgren
2008-09-24  8:00       ` Tony Lindgren
2008-09-24  8:03         ` Paul Walmsley
2008-09-24  8:06           ` Tony Lindgren
2008-09-24  8:08             ` Paul Walmsley
2008-09-24  8:34               ` Tony Lindgren
2008-09-24  9:03                 ` Arun KS
2008-09-24 14:06                 ` Paul Walmsley
2008-09-24 14:27                   ` Tony Lindgren
2008-09-24 14:35                     ` Paul Walmsley
2008-09-24 14:43                       ` Tony Lindgren

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