[2/3] xen/x86: ioapic: Rename init_ioapic_mappings() to init_ioapic()
diff mbox series

Message ID 20200327190546.21580-3-julien@xen.org
State New
Headers show
Series
  • xen/x86: Simplify ioapic_init()
Related show

Commit Message

Julien Grall March 27, 2020, 7:05 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

The function init_ioapic_mappings() is doing more than initialization
mappings. It is also initialization the number of IRQs/GSIs supported.

So rename the function to init_ioapic(). This will allow us to re-use
the name in a follow-up patch.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/x86/apic.c           | 2 +-
 xen/arch/x86/io_apic.c        | 2 +-
 xen/include/asm-x86/io_apic.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Roger Pau Monné March 30, 2020, 10:37 a.m. UTC | #1
On Fri, Mar 27, 2020 at 07:05:45PM +0000, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> The function init_ioapic_mappings() is doing more than initialization
> mappings. It is also initialization the number of IRQs/GSIs supported.
> 
> So rename the function to init_ioapic(). This will allow us to re-use
> the name in a follow-up patch.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I've got one comment/request however.

> ---
>  xen/arch/x86/apic.c           | 2 +-
>  xen/arch/x86/io_apic.c        | 2 +-
>  xen/include/asm-x86/io_apic.h | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
> index cde67cd87e..c7a54cafc3 100644
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -978,7 +978,7 @@ __next:
>          boot_cpu_physical_apicid = get_apic_id();
>      x86_cpu_to_apicid[0] = get_apic_id();
>  
> -    init_ioapic_mappings();
> +    init_ioapic();

I would rename this to ioapic_init instead since you are already
changing it. I usually prefer the subsystem name to be prefixed to the
action the function performs instead of the other way around.

Thanks, Roger.
Julien Grall March 30, 2020, 12:55 p.m. UTC | #2
On 30/03/2020 11:37, Roger Pau Monné wrote:
> On Fri, Mar 27, 2020 at 07:05:45PM +0000, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> The function init_ioapic_mappings() is doing more than initialization
>> mappings. It is also initialization the number of IRQs/GSIs supported.
>>
>> So rename the function to init_ioapic(). This will allow us to re-use
>> the name in a follow-up patch.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> I've got one comment/request however.
> 
>> ---
>>   xen/arch/x86/apic.c           | 2 +-
>>   xen/arch/x86/io_apic.c        | 2 +-
>>   xen/include/asm-x86/io_apic.h | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
>> index cde67cd87e..c7a54cafc3 100644
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -978,7 +978,7 @@ __next:
>>           boot_cpu_physical_apicid = get_apic_id();
>>       x86_cpu_to_apicid[0] = get_apic_id();
>>   
>> -    init_ioapic_mappings();
>> +    init_ioapic();
> 
> I would rename this to ioapic_init instead since you are already
> changing it. I usually prefer the subsystem name to be prefixed to the
> action the function performs instead of the other way around.

I will do it.

Cheers,
Jan Beulich March 31, 2020, 11:08 a.m. UTC | #3
On 30.03.2020 12:37, Roger Pau Monné wrote:
> On Fri, Mar 27, 2020 at 07:05:45PM +0000, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> The function init_ioapic_mappings() is doing more than initialization
>> mappings. It is also initialization the number of IRQs/GSIs supported.
>>
>> So rename the function to init_ioapic(). This will allow us to re-use
>> the name in a follow-up patch.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> I've got one comment/request however.
> 
>> ---
>>  xen/arch/x86/apic.c           | 2 +-
>>  xen/arch/x86/io_apic.c        | 2 +-
>>  xen/include/asm-x86/io_apic.h | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
>> index cde67cd87e..c7a54cafc3 100644
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -978,7 +978,7 @@ __next:
>>          boot_cpu_physical_apicid = get_apic_id();
>>      x86_cpu_to_apicid[0] = get_apic_id();
>>  
>> -    init_ioapic_mappings();
>> +    init_ioapic();
> 
> I would rename this to ioapic_init instead since you are already
> changing it. I usually prefer the subsystem name to be prefixed to the
> action the function performs instead of the other way around.

With this adjustment
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Patch
diff mbox series

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index cde67cd87e..c7a54cafc3 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -978,7 +978,7 @@  __next:
         boot_cpu_physical_apicid = get_apic_id();
     x86_cpu_to_apicid[0] = get_apic_id();
 
-    init_ioapic_mappings();
+    init_ioapic();
 }
 
 /*****************************************************************************
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 9868933287..9a11ee8342 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2537,7 +2537,7 @@  static __init bool bad_ioapic_register(unsigned int idx)
     return false;
 }
 
-void __init init_ioapic_mappings(void)
+void __init init_ioapic(void)
 {
     unsigned long ioapic_phys;
     unsigned int i, idx = FIX_IO_APIC_BASE_0;
diff --git a/xen/include/asm-x86/io_apic.h b/xen/include/asm-x86/io_apic.h
index 998905186b..8c0af4bdd3 100644
--- a/xen/include/asm-x86/io_apic.h
+++ b/xen/include/asm-x86/io_apic.h
@@ -180,7 +180,7 @@  extern int io_apic_get_version (int ioapic);
 extern int io_apic_get_redir_entries (int ioapic);
 extern int io_apic_set_pci_routing (int ioapic, int pin, int irq, int edge_level, int active_high_low);
 
-extern void init_ioapic_mappings(void);
+extern void init_ioapic(void);
 
 extern void ioapic_suspend(void);
 extern void ioapic_resume(void);