Re: [Qemu-devel] [PATCH 05/11] pxa2xx_pic: update to use qdev and arm-pic

[andrzej zaborowski]

On 11 February 2011 21:18, Dmitry Eremin-Solenikov <address@hidden> wrote:
> On 2/11/11, andrzej zaborowski <address@hidden> wrote:
>> On 31 January 2011 16:20, Dmitry Eremin-Solenikov <address@hidden>
>> wrote:
>>> pxa2xx_pic duplicated some code from arm-pic. Drop it, replacing with
>>> references to arm-pic. Also use qdev/sysbus framework to handle
>>> pxa2xx-pic.
>>
>> The duplication involves about 4 lines of code, at this level I
>> strongly prefer to not add a level of calls that can't be inlined in a
>> fast path.  I guess the choice not to involve arm_pic was conscious.
>
> I just planned to later reuse allocated arm-pic IRQ's (the new one) to
> be passed to pxa2xx-gpio (to drop usage of cpu-env). I think. I can
> still allocate
> arm-pic but use only the last IRQ from it. Will that be suitable for you?
>
>>
>> Otherwise this patch and all the remaining patches look good to me,
>> except some minor remarks:
>>
>> In patch 6 I'd prefer not to call qdev_get_gpio_in in
>> pxa2xx_rtc_int_update and similarly in patch 10 in
>
> Fixed.
>
>> mst_fpga_update_gpio, let's store the irq's in the state struct.
>
> Will inlining qdev_get_gpio_in with direct access to qdev->gpio_in[] OK?

There's a comment in hw/qdev.h that says not to do that.  I'm not sure
why qdev_get_gpio_in is not a static inline function, but it'd be
easiest to store the irq array in the mst_fpga's state.

Cheers