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

[Dmitry Eremin-Solenikov]

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?

>
> I also prefer not to make lines shorter than 80 chars longer, they
> wrap on my netbook.
>
> I pushed patches 1-3.
>
> Cheers
>


-- 
With best wishes
Dmitry