WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH 36 of 38] xen: route hardware irqs via Xen

To: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Subject: [Xen-devel] Re: [PATCH 36 of 38] xen: route hardware irqs via Xen
From: Ingo Molnar <mingo@xxxxxxx>
Date: Thu, 20 Nov 2008 10:31:59 +0100
Cc: the arch/x86 maintainers <x86@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, Ian Campbell <ian.campbell@xxxxxxxxxx>
Delivery-date: Thu, 20 Nov 2008 01:32:36 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <00079eee3818aa0205e2.1226603434@xxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1226603398@xxxxxxxxxxxxxxxxx> <00079eee3818aa0205e2.1226603434@xxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
> +#ifdef CONFIG_XEN_PCI
> +     irq = xen_register_gsi(gsi, triggering, polarity);
> +     if ((int)irq >= 0)
> +             return irq;
> +#endif

why not change irq to 'int' and avoid the cast?

also, please eliminate the #ifdef by turning xen_register_gsi() into a 
'return -1' inline on !CONFIG_XEN_PCI.

> +#ifdef CONFIG_XEN_PCI
> +     xen_pci_init();
> +#endif

hide the #ifdef in a header please. (like you already properly do for
xen_setup_pirqs())

> +     if (rc != 0) {
>               if (!probing_irq(irq))
>                       printk(KERN_INFO "Failed to obtain physical IRQ %d\n",
>                              irq);
> +             dump_stack();

generally it's better to use WARN() or WARN_ONCE() to get good debug 
feedback and stackdumps. (they also document the reason for the dump)

> @@ -523,8 +526,6 @@
>       } else
>               irq = find_unbound_irq();
>  
> -     spin_unlock(&irq_mapping_update_lock);
> -
>       set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
>                                     handle_level_irq, "pirq");

hm, looks like a stray bugfix?

        Ingo

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel