Re: [Xen-devel] [PATCH for-4.5 6/8] xen/arm: IRQ: Add lock contrainst for gic_irq_{startup, shutdown}

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.02.14 at 15:51, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Adding Keir and Jan.
> 
> On 02/19/2014 02:38 PM, Ian Campbell wrote:
>> On Wed, 2014-02-19 at 14:35 +0000, Julien Grall wrote:
>> 
>>>>> -static void gic_irq_enable(struct irq_desc *desc)
>>>>> +static unsigned int gic_irq_startup(struct irq_desc *desc)
>>>>
>>>> unsigned? What are the error codes here going to be?
>>>
>>> This is the return type requested by hw_interrupt_type.startup.
>>>
>>> It seems that the return is never checked (even in x86 code). Maybe we
>>> should change the prototype of hw_interrupt_type.startup.
>> 
>> Worth investigating. I wonder if someone thought this might return the
>> resulting interrupt number (those are normally unsigned int I think) or
>> if it actually did used to etc.
> 
> I think it was copied from Linux which also have unsigned int. I gave a
> quick look to the code and this callback is only used in 2 places which
> always return 0.
> 
> Surprisingly, the wrapper irq_startup (kernel/irq/manage.c) is returning
> an int...
> 
> I can create a patch to return void instead of unsigned if everyone is
> happy with this solution.

I'd be fine with such a change; I'd like to ask though that if you
do this, you at the same time do the resulting possible cleanup:
As an example, xen/arch/x86/msi.c:startup_msi_irq() becomes
unnecessary then. It will in fact be interesting to see how many
distinct startup routines actually remain.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel