Re: [Xen-devel] [PATCH v2 01/10] xen: remap GSIs as pirqs when running as initial domain

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Mon, 4 Oct 2010, Konrad Rzeszutek Wilk wrote:
> > +	gsi = xen_register_gsi(gsi,	trigger, polarity);
> 
> What happend here? Tabs-gone-wild?


Oops, I'll fix that.


> > +	/*
> > +	 * stash over-ride to indicate we've been here
> > +	 * and for later update of acpi_gbl_FADT
> > +	 */
> 
> I think that comment is obsolete. We are not stashing anything?

Yes, it is obsolete. I'll remove it.


> >  
> >  int __init pci_xen_init(void)
> >  {
> > -	if (!xen_pv_domain() || xen_initial_domain())
> > + 	if (!xen_pv_domain() || xen_initial_domain())
> 
> What changed? They look exactly the same?
> 

Another editing error unfortunately.


> > +void __init xen_setup_pirqs(void)
> > +{
> > +	int irq;
> > +#ifndef CONFIG_SMP
> > +	int nr_ioapics = 1;
> > +#endif
> 
> Should this be defined in a header instead? Was this nr_ioapics==1
> meant to fall in the '0 == nr_ioapics' to setup the first sixteen
> irqs?
> 
> Is CONFIG_X86_IO_APIC more appropiate than CONFIG_SMP?

I think it was supposed to fix a compilation error in case
CONFIG_X86_IO_APIC is not set, so that in both cases in which an ioapic
is not present in the system or the ioapic support is not compiled in
the kernel we would fall in the '0 == nr_ioapics' code path.
So I guess the right thing to do here would be to replace CONFIG_SMP
with CONFIG_X86_IO_APIC.
I don't think that it is worth moving these three lines into an header
file.