| Message ID | 20200327190546.21580-3-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/x86: Simplify ioapic_init() | expand |
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.
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,
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
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);