[patch 1/3] x86: apic,io-apic - generalize lapic_get_version helper

[patch 1/3] x86: apic,io-apic - generalize lapic_get_version helper

[Cyrill Gorcunov]

[-- Attachment #1: x86-apic-get-version --]
[-- Type: text/plain, Size: 1913 bytes --]

By doing so we are able to not duplicate code in
io_apic.c at least in one case.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

Index: linux-2.6.git/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic.c	2008-09-24 16:57:54.000000000 +0400
+++ linux-2.6.git/arch/x86/kernel/apic.c	2008-09-24 19:17:20.000000000 +0400
@@ -163,14 +163,6 @@ static DEFINE_PER_CPU(struct clock_event
 static unsigned long apic_phys;
 
 /*
- * Get the LAPIC version
- */
-static inline int lapic_get_version(void)
-{
-	return GET_APIC_VERSION(apic_read(APIC_LVR));
-}
-
-/*
  * Check, if the APIC is integrated or a separate chip
  */
 static inline int lapic_is_integrated(void)
Index: linux-2.6.git/arch/x86/kernel/io_apic.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/io_apic.c	2008-09-24 19:08:14.000000000 +0400
+++ linux-2.6.git/arch/x86/kernel/io_apic.c	2008-09-24 19:19:03.000000000 +0400
@@ -2759,8 +2759,7 @@ static inline void __init check_timer(vo
 
 	local_irq_save(flags);
 
-        ver = apic_read(APIC_LVR);
-        ver = GET_APIC_VERSION(ver);
+	ver = lapic_get_version();
 
 	/*
 	 * get/set the timer IRQ vector:
Index: linux-2.6.git/include/asm-x86/apic.h
===================================================================
--- linux-2.6.git.orig/include/asm-x86/apic.h	2008-09-22 17:42:35.000000000 +0400
+++ linux-2.6.git/include/asm-x86/apic.h	2008-09-24 19:16:58.000000000 +0400
@@ -132,6 +132,12 @@ extern struct apic_ops *apic_ops;
 #define apic_wait_icr_idle (apic_ops->wait_icr_idle)
 #define safe_apic_wait_icr_idle (apic_ops->safe_wait_icr_idle)
 
+/* Get the LAPIC version */
+static inline int lapic_get_version(void)
+{
+	return GET_APIC_VERSION(apic_read(APIC_LVR));
+}
+
 extern int get_physical_broadcast(void);
 
 #ifdef CONFIG_X86_64

-- 

[patch 2/3] x86: apic - simplify lapic_get_maxlvt

[Cyrill Gorcunov]

[-- Attachment #1: x86-apic-lapic_get_maxlvt-cleanup --]
[-- Type: text/plain, Size: 879 bytes --]

Lets use lapic_get_version helper and hide
register being read. Also it allow us to
make code a bit tighter.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

Index: linux-2.6.git/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic.c	2008-09-24 19:17:20.000000000 +0400
+++ linux-2.6.git/arch/x86/kernel/apic.c	2008-09-24 19:33:31.000000000 +0400
@@ -309,14 +309,14 @@ int get_physical_broadcast(void)
  */
 int lapic_get_maxlvt(void)
 {
-	unsigned int v;
+	unsigned int ver;
 
-	v = apic_read(APIC_LVR);
+	ver = lapic_get_version();
 	/*
 	 * - we always have APIC integrated on 64bit mode
 	 * - 82489DXs do not report # of LVT entries
 	 */
-	return APIC_INTEGRATED(GET_APIC_VERSION(v)) ? GET_APIC_MAXLVT(v) : 2;
+	return APIC_INTEGRATED(ver) ? GET_APIC_MAXLVT(ver) : 2;
 }
 
 /*

-- 

[patch 3/3] x86: apic - remove dead ifdef from lapic_is_integrated

[Cyrill Gorcunov]

[-- Attachment #1: x86-apic-lapic_is_integrated-cleanup --]
[-- Type: text/plain, Size: 638 bytes --]

We already have #ifdef in apicdef.h which depends
on cpu type so no need to re-introduce it.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

Index: linux-2.6.git/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic.c	2008-09-24 19:33:31.000000000 +0400
+++ linux-2.6.git/arch/x86/kernel/apic.c	2008-09-24 20:25:05.000000000 +0400
@@ -167,11 +167,7 @@ static unsigned long apic_phys;
  */
 static inline int lapic_is_integrated(void)
 {
-#ifdef CONFIG_X86_64
-	return 1;
-#else
 	return APIC_INTEGRATED(lapic_get_version());
-#endif
 }
 
 /*

-- 

Re: [patch 2/3] x86: apic - simplify lapic_get_maxlvt

[Hiroshi Shimamoto]

Cyrill Gorcunov wrote:
> Lets use lapic_get_version helper and hide
> register being read. Also it allow us to
> make code a bit tighter.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> 
> Index: linux-2.6.git/arch/x86/kernel/apic.c
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/apic.c	2008-09-24 19:17:20.000000000 +0400
> +++ linux-2.6.git/arch/x86/kernel/apic.c	2008-09-24 19:33:31.000000000 +0400
> @@ -309,14 +309,14 @@ int get_physical_broadcast(void)
>   */
>  int lapic_get_maxlvt(void)
>  {
> -	unsigned int v;
> +	unsigned int ver;
>  
> -	v = apic_read(APIC_LVR);
> +	ver = lapic_get_version();
>  	/*
>  	 * - we always have APIC integrated on 64bit mode
>  	 * - 82489DXs do not report # of LVT entries
>  	 */
> -	return APIC_INTEGRATED(GET_APIC_VERSION(v)) ? GET_APIC_MAXLVT(v) : 2;
> +	return APIC_INTEGRATED(ver) ? GET_APIC_MAXLVT(ver) : 2;
                                                      ^^^
Hi, is this correct?
Does ver have the full content of apic_read(APIC_LVR)?

thanks,
Hiroshi Shimamoto

Re: [patch 2/3] x86: apic - simplify lapic_get_maxlvt

[Yinghai Lu]

On Wed, Sep 24, 2008 at 11:58 AM, Hiroshi Shimamoto
<h-shimamoto@ct.jp.nec.com> wrote:
> Cyrill Gorcunov wrote:
>> Lets use lapic_get_version helper and hide
>> register being read. Also it allow us to
>> make code a bit tighter.
>>
>> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
>> ---
>>
>> Index: linux-2.6.git/arch/x86/kernel/apic.c
>> ===================================================================
>> --- linux-2.6.git.orig/arch/x86/kernel/apic.c 2008-09-24 19:17:20.000000000 +0400
>> +++ linux-2.6.git/arch/x86/kernel/apic.c      2008-09-24 19:33:31.000000000 +0400
>> @@ -309,14 +309,14 @@ int get_physical_broadcast(void)
>>   */
>>  int lapic_get_maxlvt(void)
>>  {
>> -     unsigned int v;
>> +     unsigned int ver;
>>
>> -     v = apic_read(APIC_LVR);
>> +     ver = lapic_get_version();
>>       /*
>>        * - we always have APIC integrated on 64bit mode
>>        * - 82489DXs do not report # of LVT entries
>>        */
>> -     return APIC_INTEGRATED(GET_APIC_VERSION(v)) ? GET_APIC_MAXLVT(v) : 2;
>> +     return APIC_INTEGRATED(ver) ? GET_APIC_MAXLVT(ver) : 2;
>                                                      ^^^
> Hi, is this correct?
> Does ver have the full content of apic_read(APIC_LVR)?


good catch..

YH

Re: [patch 2/3] x86: apic - simplify lapic_get_maxlvt

[Cyrill Gorcunov]

[Hiroshi Shimamoto - Wed, Sep 24, 2008 at 11:58:12AM -0700]
| Cyrill Gorcunov wrote:
| > Lets use lapic_get_version helper and hide
| > register being read. Also it allow us to
| > make code a bit tighter.
| > 
| > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
| > ---
| > 
| > Index: linux-2.6.git/arch/x86/kernel/apic.c
| > ===================================================================
| > --- linux-2.6.git.orig/arch/x86/kernel/apic.c	2008-09-24 19:17:20.000000000 +0400
| > +++ linux-2.6.git/arch/x86/kernel/apic.c	2008-09-24 19:33:31.000000000 +0400
| > @@ -309,14 +309,14 @@ int get_physical_broadcast(void)
| >   */
| >  int lapic_get_maxlvt(void)
| >  {
| > -	unsigned int v;
| > +	unsigned int ver;
| >  
| > -	v = apic_read(APIC_LVR);
| > +	ver = lapic_get_version();
| >  	/*
| >  	 * - we always have APIC integrated on 64bit mode
| >  	 * - 82489DXs do not report # of LVT entries
| >  	 */
| > -	return APIC_INTEGRATED(GET_APIC_VERSION(v)) ? GET_APIC_MAXLVT(v) : 2;
| > +	return APIC_INTEGRATED(ver) ? GET_APIC_MAXLVT(ver) : 2;
|                                                       ^^^
| Hi, is this correct?
| Does ver have the full content of apic_read(APIC_LVR)?
| 
| thanks,
| Hiroshi Shimamoto
| 

Hi Hiroshi,

damn, it seems I made a mistake again!
Thanks a lot, Hiroshi! Checking now!
Somehow missed this.

		- Cyrill -

Re: [patch 2/3] x86: apic - simplify lapic_get_maxlvt

[Cyrill Gorcunov]

[Hiroshi Shimamoto - Wed, Sep 24, 2008 at 11:58:12AM -0700]
| Cyrill Gorcunov wrote:
| > Lets use lapic_get_version helper and hide
| > register being read. Also it allow us to
| > make code a bit tighter.
| > 
| > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
| > ---
| > 
| > Index: linux-2.6.git/arch/x86/kernel/apic.c
| > ===================================================================
| > --- linux-2.6.git.orig/arch/x86/kernel/apic.c	2008-09-24 19:17:20.000000000 +0400
| > +++ linux-2.6.git/arch/x86/kernel/apic.c	2008-09-24 19:33:31.000000000 +0400
| > @@ -309,14 +309,14 @@ int get_physical_broadcast(void)
| >   */
| >  int lapic_get_maxlvt(void)
| >  {
| > -	unsigned int v;
| > +	unsigned int ver;
| >  
| > -	v = apic_read(APIC_LVR);
| > +	ver = lapic_get_version();
| >  	/*
| >  	 * - we always have APIC integrated on 64bit mode
| >  	 * - 82489DXs do not report # of LVT entries
| >  	 */
| > -	return APIC_INTEGRATED(GET_APIC_VERSION(v)) ? GET_APIC_MAXLVT(v) : 2;
| > +	return APIC_INTEGRATED(ver) ? GET_APIC_MAXLVT(ver) : 2;
|                                                       ^^^
| Hi, is this correct?
| Does ver have the full content of apic_read(APIC_LVR)?
| 
| thanks,
| Hiroshi Shimamoto
| 

Thanks a lot Hiroshi, I just cutted a half of
register (what a shame) :( Sorry.

Ingo, don't apply this series please!

		- Cyrill -