[PATCH 3/9 v3] omap: generic: introduce a single check_revision

[PATCH 3/9 v3] omap: generic: introduce a single check_revision

[Nishanth Menon]

Introduce a single omap generic check_revision that routes the
request to the right revision of check_revision.

Note: OMAP1 and OMAP2+ are not built into a single kernel. This
allows for the headers definitions of omap1_check_revision() and
omap2_check_revision() to be used without #ifdefs and additional cpu
checks in our single check_revision.

Cc: Tony Lindgren <tony@atomide.com>
Cc: Angelo Arrifano <miknix@gmail.com>
Cc: "Zebediah C. McClure" <zmc@lurian.net>
Cc: Alistair Buxton <a.j.buxton@gmail.com>
Cc: Grazvydas Ignotas <notasas@gmail.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Senthilvadivu Gurusamy <svadivu@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
Cc: Vikram Pandita <vikram.pandita@ti.com>
Cc: Vishwanath S <vishwa.s@ti.com>
Cc: linux-omap@vger.kernel.org

Signed-off-by: Nishanth Menon <nm@ti.com>
---
V3: comments from http://marc.info/?t=127747252000003&r=1&w=2
	fixed
V2: comments from http://marc.info/?t=127725956100006&r=1&w=2
	fixed
V1: original
 arch/arm/mach-omap1/io.c              |    3 +--
 arch/arm/mach-omap2/io.c              |    2 +-
 arch/arm/plat-omap/common.c           |    6 ++++++
 arch/arm/plat-omap/include/plat/cpu.h |   13 ++++++++++++-
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
index e4d8680..4f9ee73 100644
--- a/arch/arm/mach-omap1/io.c
+++ b/arch/arm/mach-omap1/io.c
@@ -20,7 +20,6 @@
 
 #include "clock.h"
 
-extern void omap1_check_revision(void);
 extern void omap_sram_init(void);
 
 /*
@@ -102,7 +101,7 @@ void __init omap1_map_common_io(void)
 	/* We want to check CPU revision early for cpu_is_omapxxxx() macros.
 	 * IO space mapping must be initialized before we can do that.
 	 */
-	omap1_check_revision();
+	omap_check_revision();
 
 #if defined (CONFIG_ARCH_OMAP730) || defined (CONFIG_ARCH_OMAP850)
 	if (cpu_is_omap7xx()) {
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 4e1f53d..eeb0e30 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
 	local_flush_tlb_all();
 	flush_cache_all();
 
-	omap2_check_revision();
+	omap_check_revision();
 	omap_sram_init();
 }
 
diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
index fca73cd..4a0e333 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -89,6 +89,12 @@ void __init omap_reserve(void)
 	omap_vram_reserve_sdram_lmb();
 }
 
+void __init omap_check_revision(void)
+{
+	omap1_check_revision();
+	omap2_check_revision();
+}
+
 /*
  * 32KHz clocksource ... always available, on pretty most chips except
  * OMAP 730 and 1510.  Other timers could be used as clocksources, with
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index 7514174..5f12a0b 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
 
 
 int omap_chip_is(struct omap_chip_id oci);
-void omap2_check_revision(void);
+#ifdef CONFIG_ARCH_OMAP2PLUS
+extern void omap2_check_revision(void);
+#else
+static inline void omap2_check_revision(void) {}
+#endif
+
+#ifdef CONFIG_ARCH_OMAP1
+extern void omap1_check_revision(void);
+#else
+static inline void omap1_check_revision(void) {}
+#endif
+void omap_check_revision(void);
 
 /*
  * Runtime detection of OMAP3 features
-- 
1.6.3.3

RE: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

[Shilimkar, Santosh]

> -----Original Message-----
> From: Menon, Nishanth
> Sent: Friday, June 25, 2010 9:55 PM
> To: linux-omap
> Cc: Tony Lindgren; Menon, Nishanth; Angelo Arrifano; Zebediah C. McClure; Alistair Buxton; Grazvydas
> Ignotas; Paul Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, Senthilvadivu; Kevin Hilman;
> DebBarma, Tarun Kanti; Tomi Valkeinen; Aaro Koskinen; Pandita, Vikram; S, Vishwanath; linux-
> omap@vger.kernel.org
> Subject: [PATCH 3/9 v3] omap: generic: introduce a single check_revision
> 
> Introduce a single omap generic check_revision that routes the
> request to the right revision of check_revision.
> 
> Note: OMAP1 and OMAP2+ are not built into a single kernel. This
> allows for the headers definitions of omap1_check_revision() and
> omap2_check_revision() to be used without #ifdefs and additional cpu
> checks in our single check_revision.
> 
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Angelo Arrifano <miknix@gmail.com>
> Cc: "Zebediah C. McClure" <zmc@lurian.net>
> Cc: Alistair Buxton <a.j.buxton@gmail.com>
> Cc: Grazvydas Ignotas <notasas@gmail.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Sanjeev Premi <premi@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Senthilvadivu Gurusamy <svadivu@ti.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
> Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
> Cc: Vikram Pandita <vikram.pandita@ti.com>
> Cc: Vishwanath S <vishwa.s@ti.com>
> Cc: linux-omap@vger.kernel.org
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> V3: comments from http://marc.info/?t=127747252000003&r=1&w=2
> 	fixed
> V2: comments from http://marc.info/?t=127725956100006&r=1&w=2
> 	fixed
> V1: original
>  arch/arm/mach-omap1/io.c              |    3 +--
>  arch/arm/mach-omap2/io.c              |    2 +-
>  arch/arm/plat-omap/common.c           |    6 ++++++
>  arch/arm/plat-omap/include/plat/cpu.h |   13 ++++++++++++-
>  4 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
> index e4d8680..4f9ee73 100644
> --- a/arch/arm/mach-omap1/io.c
> +++ b/arch/arm/mach-omap1/io.c
> @@ -20,7 +20,6 @@
> 
>  #include "clock.h"
> 
> -extern void omap1_check_revision(void);
>  extern void omap_sram_init(void);
> 
>  /*
> @@ -102,7 +101,7 @@ void __init omap1_map_common_io(void)
>  	/* We want to check CPU revision early for cpu_is_omapxxxx() macros.
>  	 * IO space mapping must be initialized before we can do that.
>  	 */
> -	omap1_check_revision();
> +	omap_check_revision();
> 
>  #if defined (CONFIG_ARCH_OMAP730) || defined (CONFIG_ARCH_OMAP850)
>  	if (cpu_is_omap7xx()) {
> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
> index 4e1f53d..eeb0e30 100644
> --- a/arch/arm/mach-omap2/io.c
> +++ b/arch/arm/mach-omap2/io.c
> @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
>  	local_flush_tlb_all();
>  	flush_cache_all();
> 
> -	omap2_check_revision();
> +	omap_check_revision();
>  	omap_sram_init();
>  }
> 
> diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
> index fca73cd..4a0e333 100644
> --- a/arch/arm/plat-omap/common.c
> +++ b/arch/arm/plat-omap/common.c
> @@ -89,6 +89,12 @@ void __init omap_reserve(void)
>  	omap_vram_reserve_sdram_lmb();
>  }
> 
> +void __init omap_check_revision(void)
> +{
> +	omap1_check_revision();
> +	omap2_check_revision();
> +}
> +
>  /*
>   * 32KHz clocksource ... always available, on pretty most chips except
>   * OMAP 730 and 1510.  Other timers could be used as clocksources, with
> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
> index 7514174..5f12a0b 100644
> --- a/arch/arm/plat-omap/include/plat/cpu.h
> +++ b/arch/arm/plat-omap/include/plat/cpu.h
> @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
> 
> 
>  int omap_chip_is(struct omap_chip_id oci);
> -void omap2_check_revision(void);
> +#ifdef CONFIG_ARCH_OMAP2PLUS
> +extern void omap2_check_revision(void);
> +#else
> +static inline void omap2_check_revision(void) {}
I think codingstyle suggest empty function braces to be on next line
like
static inline void omap2_check_revision(void)
{}
> +#endif
> +
> +#ifdef CONFIG_ARCH_OMAP1
> +extern void omap1_check_revision(void);
> +#else
> +static inline void omap1_check_revision(void) {}
> +#endif
> +void omap_check_revision(void);
> 
>  /*
>   * Runtime detection of OMAP3 features

Otherwise patch looks good to me. 
> --
> 1.6.3.3

Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

[Nishanth Menon]

Shilimkar, Santosh had written, on 06/25/2010 11:41 AM, the following:
>> -----Original Message-----
>> From: Menon, Nishanth
>> Sent: Friday, June 25, 2010 9:55 PM
>> To: linux-omap
>> Cc: Tony Lindgren; Menon, Nishanth; Angelo Arrifano; Zebediah C. McClure; Alistair Buxton; Grazvydas
>> Ignotas; Paul Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, Senthilvadivu; Kevin Hilman;
>> DebBarma, Tarun Kanti; Tomi Valkeinen; Aaro Koskinen; Pandita, Vikram; S, Vishwanath; linux-
>> omap@vger.kernel.org
>> Subject: [PATCH 3/9 v3] omap: generic: introduce a single check_revision
>>
>> Introduce a single omap generic check_revision that routes the
>> request to the right revision of check_revision.
>>
>> Note: OMAP1 and OMAP2+ are not built into a single kernel. This
>> allows for the headers definitions of omap1_check_revision() and
>> omap2_check_revision() to be used without #ifdefs and additional cpu
>> checks in our single check_revision.
>>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Angelo Arrifano <miknix@gmail.com>
>> Cc: "Zebediah C. McClure" <zmc@lurian.net>
>> Cc: Alistair Buxton <a.j.buxton@gmail.com>
>> Cc: Grazvydas Ignotas <notasas@gmail.com>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Sanjeev Premi <premi@ti.com>
>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Senthilvadivu Gurusamy <svadivu@ti.com>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
>> Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
>> Cc: Vikram Pandita <vikram.pandita@ti.com>
>> Cc: Vishwanath S <vishwa.s@ti.com>
>> Cc: linux-omap@vger.kernel.org
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>> V3: comments from http://marc.info/?t=127747252000003&r=1&w=2
>> 	fixed
>> V2: comments from http://marc.info/?t=127725956100006&r=1&w=2
>> 	fixed
>> V1: original
>>  arch/arm/mach-omap1/io.c              |    3 +--
>>  arch/arm/mach-omap2/io.c              |    2 +-
>>  arch/arm/plat-omap/common.c           |    6 ++++++
>>  arch/arm/plat-omap/include/plat/cpu.h |   13 ++++++++++++-
>>  4 files changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
>> index e4d8680..4f9ee73 100644
>> --- a/arch/arm/mach-omap1/io.c
>> +++ b/arch/arm/mach-omap1/io.c
>> @@ -20,7 +20,6 @@
>>
>>  #include "clock.h"
>>
>> -extern void omap1_check_revision(void);
>>  extern void omap_sram_init(void);
>>
>>  /*
>> @@ -102,7 +101,7 @@ void __init omap1_map_common_io(void)
>>  	/* We want to check CPU revision early for cpu_is_omapxxxx() macros.
>>  	 * IO space mapping must be initialized before we can do that.
>>  	 */
>> -	omap1_check_revision();
>> +	omap_check_revision();
>>
>>  #if defined (CONFIG_ARCH_OMAP730) || defined (CONFIG_ARCH_OMAP850)
>>  	if (cpu_is_omap7xx()) {
>> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
>> index 4e1f53d..eeb0e30 100644
>> --- a/arch/arm/mach-omap2/io.c
>> +++ b/arch/arm/mach-omap2/io.c
>> @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
>>  	local_flush_tlb_all();
>>  	flush_cache_all();
>>
>> -	omap2_check_revision();
>> +	omap_check_revision();
>>  	omap_sram_init();
>>  }
>>
>> diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
>> index fca73cd..4a0e333 100644
>> --- a/arch/arm/plat-omap/common.c
>> +++ b/arch/arm/plat-omap/common.c
>> @@ -89,6 +89,12 @@ void __init omap_reserve(void)
>>  	omap_vram_reserve_sdram_lmb();
>>  }
>>
>> +void __init omap_check_revision(void)
>> +{
>> +	omap1_check_revision();
>> +	omap2_check_revision();
>> +}
>> +
>>  /*
>>   * 32KHz clocksource ... always available, on pretty most chips except
>>   * OMAP 730 and 1510.  Other timers could be used as clocksources, with
>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
>> index 7514174..5f12a0b 100644
>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>> @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
>>
>>
>>  int omap_chip_is(struct omap_chip_id oci);
>> -void omap2_check_revision(void);
>> +#ifdef CONFIG_ARCH_OMAP2PLUS
>> +extern void omap2_check_revision(void);
>> +#else
>> +static inline void omap2_check_revision(void) {}
> I think codingstyle suggest empty function braces to be on next line
> like
> static inline void omap2_check_revision(void)
> {}

are you sure about that? can you point me to the documentation for that?
Style I followed is off:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=72651f788f4e3536149ef5e7ddfbed96a8f14d2f;hb=HEAD#l661

>> +#endif
>> +
>> +#ifdef CONFIG_ARCH_OMAP1
>> +extern void omap1_check_revision(void);
>> +#else
>> +static inline void omap1_check_revision(void) {}
>> +#endif
>> +void omap_check_revision(void);
>>
>>  /*
>>   * Runtime detection of OMAP3 features
> 
> Otherwise patch looks good to me. 
thanks for the ack.

>> --
>> 1.6.3.3
> 


-- 
Regards,
Nishanth Menon

RE: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

[Shilimkar, Santosh]

> -----Original Message-----
> From: Menon, Nishanth
> Sent: Friday, June 25, 2010 11:02 PM
> To: Shilimkar, Santosh
> Cc: linux-omap; Tony Lindgren; Angelo Arrifano; Zebediah C. McClure; Alistair Buxton; Grazvydas
> Ignotas; Paul Walmsley; Premi, Sanjeev; Guruswamy, Senthilvadivu; Kevin Hilman; DebBarma, Tarun
> Kanti; Tomi Valkeinen; Aaro Koskinen; Pandita, Vikram; S, Vishwanath
> Subject: Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision
> 
> Shilimkar, Santosh had written, on 06/25/2010 11:41 AM, the following:
> >> -----Original Message-----
> >> From: Menon, Nishanth
> >> Sent: Friday, June 25, 2010 9:55 PM
> >> To: linux-omap
> >> Cc: Tony Lindgren; Menon, Nishanth; Angelo Arrifano; Zebediah C. McClure; Alistair Buxton;
> Grazvydas
> >> Ignotas; Paul Walmsley; Premi, Sanjeev; Shilimkar, Santosh; Guruswamy, Senthilvadivu; Kevin
> Hilman;
> >> DebBarma, Tarun Kanti; Tomi Valkeinen; Aaro Koskinen; Pandita, Vikram; S, Vishwanath; linux-
> >> omap@vger.kernel.org
> >> Subject: [PATCH 3/9 v3] omap: generic: introduce a single check_revision
> >>
> >> Introduce a single omap generic check_revision that routes the
> >> request to the right revision of check_revision.
> >>
> >> Note: OMAP1 and OMAP2+ are not built into a single kernel. This
> >> allows for the headers definitions of omap1_check_revision() and
> >> omap2_check_revision() to be used without #ifdefs and additional cpu
> >> checks in our single check_revision.
> >>
> >> Cc: Tony Lindgren <tony@atomide.com>
> >> Cc: Angelo Arrifano <miknix@gmail.com>
> >> Cc: "Zebediah C. McClure" <zmc@lurian.net>
> >> Cc: Alistair Buxton <a.j.buxton@gmail.com>
> >> Cc: Grazvydas Ignotas <notasas@gmail.com>
> >> Cc: Paul Walmsley <paul@pwsan.com>
> >> Cc: Sanjeev Premi <premi@ti.com>
> >> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >> Cc: Senthilvadivu Gurusamy <svadivu@ti.com>
> >> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> >> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> >> Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
> >> Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
> >> Cc: Vikram Pandita <vikram.pandita@ti.com>
> >> Cc: Vishwanath S <vishwa.s@ti.com>
> >> Cc: linux-omap@vger.kernel.org
> >>
> >> Signed-off-by: Nishanth Menon <nm@ti.com>
> >> ---
> >> V3: comments from http://marc.info/?t=127747252000003&r=1&w=2
> >> 	fixed
> >> V2: comments from http://marc.info/?t=127725956100006&r=1&w=2
> >> 	fixed
> >> V1: original
> >>  arch/arm/mach-omap1/io.c              |    3 +--
> >>  arch/arm/mach-omap2/io.c              |    2 +-
> >>  arch/arm/plat-omap/common.c           |    6 ++++++
> >>  arch/arm/plat-omap/include/plat/cpu.h |   13 ++++++++++++-
> >>  4 files changed, 20 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
> >> index e4d8680..4f9ee73 100644
> >> --- a/arch/arm/mach-omap1/io.c
> >> +++ b/arch/arm/mach-omap1/io.c
> >> @@ -20,7 +20,6 @@
> >>
> >>  #include "clock.h"
> >>
> >> -extern void omap1_check_revision(void);
> >>  extern void omap_sram_init(void);
> >>
> >>  /*
> >> @@ -102,7 +101,7 @@ void __init omap1_map_common_io(void)
> >>  	/* We want to check CPU revision early for cpu_is_omapxxxx() macros.
> >>  	 * IO space mapping must be initialized before we can do that.
> >>  	 */
> >> -	omap1_check_revision();
> >> +	omap_check_revision();
> >>
> >>  #if defined (CONFIG_ARCH_OMAP730) || defined (CONFIG_ARCH_OMAP850)
> >>  	if (cpu_is_omap7xx()) {
> >> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
> >> index 4e1f53d..eeb0e30 100644
> >> --- a/arch/arm/mach-omap2/io.c
> >> +++ b/arch/arm/mach-omap2/io.c
> >> @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
> >>  	local_flush_tlb_all();
> >>  	flush_cache_all();
> >>
> >> -	omap2_check_revision();
> >> +	omap_check_revision();
> >>  	omap_sram_init();
> >>  }
> >>
> >> diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
> >> index fca73cd..4a0e333 100644
> >> --- a/arch/arm/plat-omap/common.c
> >> +++ b/arch/arm/plat-omap/common.c
> >> @@ -89,6 +89,12 @@ void __init omap_reserve(void)
> >>  	omap_vram_reserve_sdram_lmb();
> >>  }
> >>
> >> +void __init omap_check_revision(void)
> >> +{
> >> +	omap1_check_revision();
> >> +	omap2_check_revision();
> >> +}
> >> +
> >>  /*
> >>   * 32KHz clocksource ... always available, on pretty most chips except
> >>   * OMAP 730 and 1510.  Other timers could be used as clocksources, with
> >> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
> >> index 7514174..5f12a0b 100644
> >> --- a/arch/arm/plat-omap/include/plat/cpu.h
> >> +++ b/arch/arm/plat-omap/include/plat/cpu.h
> >> @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
> >>
> >>
> >>  int omap_chip_is(struct omap_chip_id oci);
> >> -void omap2_check_revision(void);
> >> +#ifdef CONFIG_ARCH_OMAP2PLUS
> >> +extern void omap2_check_revision(void);
> >> +#else
> >> +static inline void omap2_check_revision(void) {}
> > I think codingstyle suggest empty function braces to be on next line
> > like
> > static inline void omap2_check_revision(void)
> > {}
> 
> are you sure about that? can you point me to the documentation for that?
> Style I followed is off:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
> 2.6.git;a=blob;f=Documentation/SubmittingPatches;h=72651f788f4e3536149ef5e7ddfbed96a8f14d2f;hb=HEAD#l
> 661
> 
I got similar comment long back and hence remembered. Looks like it's not explicitly documented
> >> +#endif
> >> +
> >> +#ifdef CONFIG_ARCH_OMAP1
> >> +extern void omap1_check_revision(void);
> >> +#else
> >> +static inline void omap1_check_revision(void) {}
> >> +#endif
> >> +void omap_check_revision(void);
> >>
> >>  /*
> >>   * Runtime detection of OMAP3 features
> >
> > Otherwise patch looks good to me.
> thanks for the ack.
> 
> >> --
> >> 1.6.3.3
> >
> 
> 
> --
> Regards,
> Nishanth Menon

static inline function style (was Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision)

[Nishanth Menon]

Shilimkar, Santosh had written, on 06/25/2010 01:07 PM, the following:
>> -----Original Message-----
>> From: Menon, Nishanth
>> Sent: Friday, June 25, 2010 11:02 PM

[..]

>>>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>>>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>>>> @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
>>>>
>>>>
>>>>  int omap_chip_is(struct omap_chip_id oci);
>>>> -void omap2_check_revision(void);
>>>> +#ifdef CONFIG_ARCH_OMAP2PLUS
>>>> +extern void omap2_check_revision(void);
>>>> +#else
>>>> +static inline void omap2_check_revision(void) {}
>>> I think codingstyle suggest empty function braces to be on next line
>>> like
>>> static inline void omap2_check_revision(void)
>>> {}
>> are you sure about that? can you point me to the documentation for that?
>> Style I followed is off:
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
>> 2.6.git;a=blob;f=Documentation/SubmittingPatches;h=72651f788f4e3536149ef5e7ddfbed96a8f14d2f;hb=HEAD#l
>> 661
>>
> I got similar comment long back and hence remembered. Looks like it's not explicitly documented
changing subject to get folks interested..

i would think that checkpatch should crib about it, but it being an 
automated script, could be messed up sometimes..

but I am curious -
static inline void foo(void) { }
static inline int foo(void)
{
	return -ENODEV;
}

is the style I have seen to date. usually without a functional code, it 
made more sense to have it out of line and more in the style of a normal 
function..

could someone give any suggestions on this?
-- 
Regards,
Nishanth Menon

Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [100625 19:19]:
> --- a/arch/arm/mach-omap2/io.c
> +++ b/arch/arm/mach-omap2/io.c
> @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
>  	local_flush_tlb_all();
>  	flush_cache_all();
>  
> -	omap2_check_revision();
> +	omap_check_revision();
>  	omap_sram_init();
>  }
>  
> diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
> index fca73cd..4a0e333 100644
> --- a/arch/arm/plat-omap/common.c
> +++ b/arch/arm/plat-omap/common.c
> @@ -89,6 +89,12 @@ void __init omap_reserve(void)
>  	omap_vram_reserve_sdram_lmb();
>  }
>  
> +void __init omap_check_revision(void)
> +{
> +	omap1_check_revision();
> +	omap2_check_revision();
> +}
> +
>  /*
>   * 32KHz clocksource ... always available, on pretty most chips except
>   * OMAP 730 and 1510.  Other timers could be used as clocksources, with
> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
> index 7514174..5f12a0b 100644
> --- a/arch/arm/plat-omap/include/plat/cpu.h
> +++ b/arch/arm/plat-omap/include/plat/cpu.h
> @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
>  
>  
>  int omap_chip_is(struct omap_chip_id oci);
> -void omap2_check_revision(void);
> +#ifdef CONFIG_ARCH_OMAP2PLUS
> +extern void omap2_check_revision(void);
> +#else
> +static inline void omap2_check_revision(void) {}
> +#endif
> +
> +#ifdef CONFIG_ARCH_OMAP1
> +extern void omap1_check_revision(void);
> +#else
> +static inline void omap1_check_revision(void) {}
> +#endif
> +void omap_check_revision(void);

Hmm, to me it seems like we should have static omap_check_revision
in both mach-omap1/id.c and mach-omap2/id.c. Or do we need to call
these anywhere else outside both id.c files?

Then these can set u32 omap_revision flags in plat-omap/common.c,
and then we can have a common omap_get_revision() or something
in plat-omap/common.c?

There should not be need for cpu_is_omapxxxx tests for getting
the revision after it's initialized.

Regards,

Tony

Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

[Nishanth Menon]

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

Tony Lindgren had written, on 07/07/2010 07:36 AM, the following:
> * Nishanth Menon <nm@ti.com> [100625 19:19]:
>> --- a/arch/arm/mach-omap2/io.c
>> +++ b/arch/arm/mach-omap2/io.c
>> @@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
>>  	local_flush_tlb_all();
>>  	flush_cache_all();
>>  
>> -	omap2_check_revision();
>> +	omap_check_revision();
>>  	omap_sram_init();
>>  }
>>  
>> diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
>> index fca73cd..4a0e333 100644
>> --- a/arch/arm/plat-omap/common.c
>> +++ b/arch/arm/plat-omap/common.c
>> @@ -89,6 +89,12 @@ void __init omap_reserve(void)
>>  	omap_vram_reserve_sdram_lmb();
>>  }
>>  
>> +void __init omap_check_revision(void)
>> +{
>> +	omap1_check_revision();
>> +	omap2_check_revision();
>> +}
>> +
>>  /*
>>   * 32KHz clocksource ... always available, on pretty most chips except
>>   * OMAP 730 and 1510.  Other timers could be used as clocksources, with
>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
>> index 7514174..5f12a0b 100644
>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>> @@ -431,7 +431,18 @@ IS_OMAP_TYPE(3517, 0x3517)
>>  
>>  
>>  int omap_chip_is(struct omap_chip_id oci);
>> -void omap2_check_revision(void);
>> +#ifdef CONFIG_ARCH_OMAP2PLUS
>> +extern void omap2_check_revision(void);
>> +#else
>> +static inline void omap2_check_revision(void) {}
>> +#endif
>> +
>> +#ifdef CONFIG_ARCH_OMAP1
>> +extern void omap1_check_revision(void);
>> +#else
>> +static inline void omap1_check_revision(void) {}
>> +#endif
>> +void omap_check_revision(void);
> 
> Hmm, to me it seems like we should have static omap_check_revision
> in both mach-omap1/id.c and mach-omap2/id.c. Or do we need to call
> these anywhere else outside both id.c files?
check_revision is called from mach-omap[12]/io.c - so no chance of the 
check_revision to be static..

> 
> Then these can set u32 omap_revision flags in plat-omap/common.c,
> and then we can have a common omap_get_revision() or something
> in plat-omap/common.c?
i think I managed to get rid of it entirely.. ref: attached patch

If we are ok with this, I will repost the series (i squashed 
omap1-rename-check_revision into this patch).

> 
> There should not be need for cpu_is_omapxxxx tests for getting
> the revision after it's initialized.
I am not sure.. if you would like drivers to be modprobabe, there may be 
quirks that you'd want to enable based on cpu_is_omapxxx checks. so it 
probably does not make sense to __initdata the revision/feature variables.


-- 
Regards,
Nishanth Menon

[-- Attachment #2: 0002-omap-generic-introduce-a-single-check_revision.patch --]
[-- Type: text/x-patch, Size: 2843 bytes --]

>From f72070e575433ad07ed018aef5c43677424003d0 Mon Sep 17 00:00:00 2001
From: Nishanth Menon <nm@ti.com>
Date: Fri, 21 May 2010 12:09:33 -0500
Subject: [PATCH 2/7] omap: generic: introduce a single check_revision

Introduce a single omap generic check_revision that routes the
request to the right revision of check_revision.

Note: OMAP1 and OMAP2+ are not built into a single kernel.

Cc: Tony Lindgren <tony@atomide.com>
Cc: Angelo Arrifano <miknix@gmail.com>
Cc: "Zebediah C. McClure" <zmc@lurian.net>
Cc: Alistair Buxton <a.j.buxton@gmail.com>
Cc: Grazvydas Ignotas <notasas@gmail.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Senthilvadivu Gurusamy <svadivu@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
Cc: Vikram Pandita <vikram.pandita@ti.com>
Cc: Vishwanath S <vishwa.s@ti.com>
Cc: linux-omap@vger.kernel.org

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap1/io.c              |    1 -
 arch/arm/mach-omap2/id.c              |    2 +-
 arch/arm/mach-omap2/io.c              |    2 +-
 arch/arm/plat-omap/include/plat/cpu.h |    3 ++-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap1/io.c b/arch/arm/mach-omap1/io.c
index 0ce3fec..4f9ee73 100644
--- a/arch/arm/mach-omap1/io.c
+++ b/arch/arm/mach-omap1/io.c
@@ -20,7 +20,6 @@
 
 #include "clock.h"
 
-extern void omap_check_revision(void);
 extern void omap_sram_init(void);
 
 /*
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index c7bf0e1..80f0950 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -371,7 +371,7 @@ static void __init omap3_cpuinfo(void)
 /*
  * Try to detect the exact revision of the omap we're running on
  */
-void __init omap2_check_revision(void)
+void __init omap_check_revision(void)
 {
 	/*
 	 * At this point we have an idea about the processor revision set
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index b9ea70b..75883fe 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -238,7 +238,7 @@ static void __init _omap2_map_common_io(void)
 	local_flush_tlb_all();
 	flush_cache_all();
 
-	omap2_check_revision();
+	omap_check_revision();
 	omap_sram_init();
 }
 
diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h
index 7514174..d25ba40 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -431,7 +431,8 @@ IS_OMAP_TYPE(3517, 0x3517)
 
 
 int omap_chip_is(struct omap_chip_id oci);
-void omap2_check_revision(void);
+
+void omap_check_revision(void);
 
 /*
  * Runtime detection of OMAP3 features
-- 
1.6.3.3

Re: [PATCH 3/9 v3] omap: generic: introduce a single check_revision

[Felipe Balbi]

Hi,

On Wed, Jul 07, 2010 at 07:24:16PM +0200, ext Nishanth Menon wrote:
>I am not sure.. if you would like drivers to be modprobabe, there may be
>quirks that you'd want to enable based on cpu_is_omapxxx checks. so it
>probably does not make sense to __initdata the revision/feature variables.

can't you pass the quirks via pdata, then ?

-- 
balbi

DefectiveByDesign.org