On 11 February 2011 21:18, Dmitry Eremin-Solenikov <dbarysh...@gmail.com> wrote: > On 2/11/11, andrzej zaborowski <balr...@gmail.com> wrote: >> On 31 January 2011 16:20, Dmitry Eremin-Solenikov <dbarysh...@gmail.com> >> 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