Message ID | 1298209838-27699-3-git-send-email-dbaryshkov@gmail.com |
---|---|
State | New |
Headers | show |
Hi Dmitry, On 20 February 2011 14:50, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote: > Use qdev/sysbus framework to handle pxa2xx-pic. Instead of exposing IRQs > via array, reference them via qdev_get_gpio_in(). Also pxa2xx_pic duplicated > some code from arm-pic. Drop it, replacing with references to arm-pic, > as all other ARM SoCs do for their PIC code. As I said earlier not using arm-pic was deliberate (and I also asked what the gain was from converting the pic to a separate sysbus device from the CPU) so I skipped this part of the patch and pushed the rest of it, please check that everything works. > > Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> > --- > hw/mainstone.c | 2 +- > hw/pxa.h | 12 +++-- > hw/pxa2xx.c | 84 +++++++++++++++++++++++------------ > hw/pxa2xx_gpio.c | 11 +++-- > hw/pxa2xx_pic.c | 126 ++++++++++++++++++++++++++++------------------------- > hw/pxa2xx_timer.c | 16 +++--- > 6 files changed, 144 insertions(+), 107 deletions(-) > > diff --git a/hw/mainstone.c b/hw/mainstone.c > index aec8d34..4eabdb9 100644 > --- a/hw/mainstone.c > +++ b/hw/mainstone.c > @@ -140,7 +140,7 @@ static void mainstone_common_init(ram_addr_t ram_size, > } > > mst_irq = sysbus_create_simple("mainstone-fpga", MST_FPGA_PHYS, > - cpu->pic[PXA2XX_PIC_GPIO_0]); > + qdev_get_gpio_in(cpu->pic, PXA2XX_PIC_GPIO_0)); I'm also wondering if this device should really use the interrupt line instead of using a GPIO. It seems wrong that both the fpga and the gpio module are connected to the same line. > > /* setup keypad */ > printf("map addr %p\n", &map); > diff --git a/hw/pxa.h b/hw/pxa.h > index f73d33b..7c6fd44 100644 > --- a/hw/pxa.h > +++ b/hw/pxa.h > @@ -63,15 +63,16 @@ > # define PXA2XX_INTERNAL_SIZE 0x40000 > > /* pxa2xx_pic.c */ > -qemu_irq *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env); > +DeviceState *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env, > + qemu_irq *arm_pic); > > /* pxa2xx_timer.c */ > -void pxa25x_timer_init(target_phys_addr_t base, qemu_irq *irqs); > -void pxa27x_timer_init(target_phys_addr_t base, qemu_irq *irqs, qemu_irq irq4); > +void pxa25x_timer_init(target_phys_addr_t base, DeviceState *pic); > +void pxa27x_timer_init(target_phys_addr_t base, DeviceState *pic); > > /* pxa2xx_gpio.c */ > DeviceState *pxa2xx_gpio_init(target_phys_addr_t base, > - CPUState *env, qemu_irq *pic, int lines); > + CPUState *env, DeviceState *pic, int lines); > void pxa2xx_gpio_read_notifier(DeviceState *dev, qemu_irq handler); > > /* pxa2xx_dma.c */ > @@ -125,7 +126,7 @@ typedef struct PXA2xxFIrState PXA2xxFIrState; > > typedef struct { > CPUState *env; > - qemu_irq *pic; > + DeviceState *pic; > qemu_irq reset; > PXA2xxDMAState *dma; > DeviceState *gpio; > @@ -180,6 +181,7 @@ typedef struct { > QEMUTimer *rtc_swal1; > QEMUTimer *rtc_swal2; > QEMUTimer *rtc_pi; > + qemu_irq rtc_irq; > } PXA2xxState; > > struct PXA2xxI2SState { > diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c > index 9ebbce6..58e6e7b 100644 > --- a/hw/pxa2xx.c > +++ b/hw/pxa2xx.c > @@ -16,6 +16,7 @@ > #include "qemu-timer.h" > #include "qemu-char.h" > #include "blockdev.h" > +#include "arm-misc.h" > > static struct { > target_phys_addr_t io_base; > @@ -888,7 +889,7 @@ static int pxa2xx_ssp_init(SysBusDevice *dev) > > static inline void pxa2xx_rtc_int_update(PXA2xxState *s) > { > - qemu_set_irq(s->pic[PXA2XX_PIC_RTCALARM], !!(s->rtsr & 0x2553)); > + qemu_set_irq(s->rtc_irq, !!(s->rtsr & 0x2553)); > } > > static void pxa2xx_rtc_hzupdate(PXA2xxState *s) > @@ -1197,6 +1198,8 @@ static void pxa2xx_rtc_init(PXA2xxState *s) > s->rtc_swal1 = qemu_new_timer(rt_clock, pxa2xx_rtc_swal1_tick, s); > s->rtc_swal2 = qemu_new_timer(rt_clock, pxa2xx_rtc_swal2_tick, s); > s->rtc_pi = qemu_new_timer(rt_clock, pxa2xx_rtc_pi_tick, s); > + > + s->rtc_irq = qdev_get_gpio_in(s->pic, PXA2XX_PIC_RTCALARM); > } > > static void pxa2xx_rtc_save(QEMUFile *f, void *opaque) > @@ -2069,6 +2072,8 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) > PXA2xxState *s; > int iomemtype, i; > DriveInfo *dinfo; > + qemu_irq *arm_pic; > + > s = (PXA2xxState *) qemu_mallocz(sizeof(PXA2xxState)); > > if (revision && strncmp(revision, "pxa27", 5)) { > @@ -2093,12 +2098,13 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) > 0x40000, qemu_ram_alloc(NULL, "pxa270.internal", > 0x40000) | IO_MEM_RAM); > > - s->pic = pxa2xx_pic_init(0x40d00000, s->env); > + arm_pic = arm_pic_init_cpu(s->env); > + s->pic = pxa2xx_pic_init(0x40d00000, s->env, arm_pic); > > - s->dma = pxa27x_dma_init(0x40000000, s->pic[PXA2XX_PIC_DMA]); > + s->dma = pxa27x_dma_init(0x40000000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_DMA)); > > - pxa27x_timer_init(0x40a00000, &s->pic[PXA2XX_PIC_OST_0], > - s->pic[PXA27X_PIC_OST_4_11]); > + pxa27x_timer_init(0x40a00000, s->pic); > > s->gpio = pxa2xx_gpio_init(0x40e00000, s->env, s->pic, 121); > > @@ -2108,26 +2114,30 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) > exit(1); > } > s->mmc = pxa2xx_mmci_init(0x41100000, dinfo->bdrv, > - s->pic[PXA2XX_PIC_MMC], s->dma); > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC), s->dma); > > for (i = 0; pxa270_serial[i].io_base; i ++) > if (serial_hds[i]) > #ifdef TARGET_WORDS_BIGENDIAN > serial_mm_init(pxa270_serial[i].io_base, 2, > - s->pic[pxa270_serial[i].irqn], 14857000/16, > + qdev_get_gpio_in(s->pic, pxa270_serial[i].irqn), > + 14857000/16, > serial_hds[i], 1, 1); > #else > serial_mm_init(pxa270_serial[i].io_base, 2, > - s->pic[pxa270_serial[i].irqn], 14857000/16, > + qdev_get_gpio_in(s->pic, pxa270_serial[i].irqn), > + 14857000/16, > serial_hds[i], 1, 0); > #endif > else > break; > if (serial_hds[i]) > - s->fir = pxa2xx_fir_init(0x40800000, s->pic[PXA2XX_PIC_ICP], > + s->fir = pxa2xx_fir_init(0x40800000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_ICP), > s->dma, serial_hds[i]); > > - s->lcd = pxa2xx_lcdc_init(0x44000000, s->pic[PXA2XX_PIC_LCD]); > + s->lcd = pxa2xx_lcdc_init(0x44000000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_LCD)); > > s->cm_base = 0x41300000; > s->cm_regs[CCCR >> 2] = 0x02000210; /* 416.0 MHz */ > @@ -2159,13 +2169,13 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) > for (i = 0; pxa27x_ssp[i].io_base; i ++) { > DeviceState *dev; > dev = sysbus_create_simple("pxa2xx-ssp", pxa27x_ssp[i].io_base, > - s->pic[pxa27x_ssp[i].irqn]); > + qdev_get_gpio_in(s->pic, pxa27x_ssp[i].irqn)); > s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); > } > > if (usb_enabled) { > sysbus_create_simple("sysbus-ohci", 0x4c000000, > - s->pic[PXA2XX_PIC_USBH1]); > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); > } > > s->pcmcia[0] = pxa2xx_pcmcia_init(0x20000000); > @@ -2179,12 +2189,17 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) > register_savevm(NULL, "pxa2xx_rtc", 0, 0, pxa2xx_rtc_save, > pxa2xx_rtc_load, s); > > - s->i2c[0] = pxa2xx_i2c_init(0x40301600, s->pic[PXA2XX_PIC_I2C], 0xffff); > - s->i2c[1] = pxa2xx_i2c_init(0x40f00100, s->pic[PXA2XX_PIC_PWRI2C], 0xff); > + s->i2c[0] = pxa2xx_i2c_init(0x40301600, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_I2C), 0xffff); > + s->i2c[1] = pxa2xx_i2c_init(0x40f00100, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_PWRI2C), 0xff); > > - s->i2s = pxa2xx_i2s_init(0x40400000, s->pic[PXA2XX_PIC_I2S], s->dma); > + s->i2s = pxa2xx_i2s_init(0x40400000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_I2S), > + s->dma); > > - s->kp = pxa27x_keypad_init(0x41500000, s->pic[PXA2XX_PIC_KEYPAD]); > + s->kp = pxa27x_keypad_init(0x41500000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_KEYPAD)); > > /* GPIO1 resets the processor */ > /* The handler can be overridden by board-specific code */ > @@ -2198,6 +2213,7 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) > PXA2xxState *s; > int iomemtype, i; > DriveInfo *dinfo; > + qemu_irq *arm_pic; > > s = (PXA2xxState *) qemu_mallocz(sizeof(PXA2xxState)); > > @@ -2216,11 +2232,13 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) > qemu_ram_alloc(NULL, "pxa255.internal", > PXA2XX_INTERNAL_SIZE) | IO_MEM_RAM); > > - s->pic = pxa2xx_pic_init(0x40d00000, s->env); > + arm_pic = arm_pic_init_cpu(s->env); > + s->pic = pxa2xx_pic_init(0x40d00000, s->env, arm_pic); > > - s->dma = pxa255_dma_init(0x40000000, s->pic[PXA2XX_PIC_DMA]); > + s->dma = pxa255_dma_init(0x40000000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_DMA)); > > - pxa25x_timer_init(0x40a00000, &s->pic[PXA2XX_PIC_OST_0]); > + pxa25x_timer_init(0x40a00000, s->pic); > > s->gpio = pxa2xx_gpio_init(0x40e00000, s->env, s->pic, 85); > > @@ -2230,27 +2248,31 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) > exit(1); > } > s->mmc = pxa2xx_mmci_init(0x41100000, dinfo->bdrv, > - s->pic[PXA2XX_PIC_MMC], s->dma); > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC), s->dma); > > for (i = 0; pxa255_serial[i].io_base; i ++) > if (serial_hds[i]) { > #ifdef TARGET_WORDS_BIGENDIAN > serial_mm_init(pxa255_serial[i].io_base, 2, > - s->pic[pxa255_serial[i].irqn], 14745600/16, > + qdev_get_gpio_in(s->pic, pxa255_serial[i].irqn), > + 14745600/16, > serial_hds[i], 1, 1); > #else > serial_mm_init(pxa255_serial[i].io_base, 2, > - s->pic[pxa255_serial[i].irqn], 14745600/16, > + qdev_get_gpio_in(s->pic, pxa255_serial[i].irqn), > + 14745600/16, > serial_hds[i], 1, 0); > #endif > } else { > break; > } > if (serial_hds[i]) > - s->fir = pxa2xx_fir_init(0x40800000, s->pic[PXA2XX_PIC_ICP], > + s->fir = pxa2xx_fir_init(0x40800000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_ICP), > s->dma, serial_hds[i]); > > - s->lcd = pxa2xx_lcdc_init(0x44000000, s->pic[PXA2XX_PIC_LCD]); > + s->lcd = pxa2xx_lcdc_init(0x44000000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_LCD)); > > s->cm_base = 0x41300000; > s->cm_regs[CCCR >> 2] = 0x02000210; /* 416.0 MHz */ > @@ -2282,13 +2304,13 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) > for (i = 0; pxa255_ssp[i].io_base; i ++) { > DeviceState *dev; > dev = sysbus_create_simple("pxa2xx-ssp", pxa255_ssp[i].io_base, > - s->pic[pxa255_ssp[i].irqn]); > + qdev_get_gpio_in(s->pic, pxa255_ssp[i].irqn)); > s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); > } > > if (usb_enabled) { > sysbus_create_simple("sysbus-ohci", 0x4c000000, > - s->pic[PXA2XX_PIC_USBH1]); > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); > } > > s->pcmcia[0] = pxa2xx_pcmcia_init(0x20000000); > @@ -2302,10 +2324,14 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) > register_savevm(NULL, "pxa2xx_rtc", 0, 0, pxa2xx_rtc_save, > pxa2xx_rtc_load, s); > > - s->i2c[0] = pxa2xx_i2c_init(0x40301600, s->pic[PXA2XX_PIC_I2C], 0xffff); > - s->i2c[1] = pxa2xx_i2c_init(0x40f00100, s->pic[PXA2XX_PIC_PWRI2C], 0xff); > + s->i2c[0] = pxa2xx_i2c_init(0x40301600, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_I2C), 0xffff); > + s->i2c[1] = pxa2xx_i2c_init(0x40f00100, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_PWRI2C), 0xff); > > - s->i2s = pxa2xx_i2s_init(0x40400000, s->pic[PXA2XX_PIC_I2S], s->dma); > + s->i2s = pxa2xx_i2s_init(0x40400000, > + qdev_get_gpio_in(s->pic, PXA2XX_PIC_I2S), > + s->dma); > > /* GPIO1 resets the processor */ > /* The handler can be overridden by board-specific code */ > diff --git a/hw/pxa2xx_gpio.c b/hw/pxa2xx_gpio.c > index 789965d..16a8865 100644 > --- a/hw/pxa2xx_gpio.c > +++ b/hw/pxa2xx_gpio.c > @@ -253,7 +253,7 @@ static CPUWriteMemoryFunc * const pxa2xx_gpio_writefn[] = { > }; > > DeviceState *pxa2xx_gpio_init(target_phys_addr_t base, > - CPUState *env, qemu_irq *pic, int lines) > + CPUState *env, DeviceState *pic, int lines) > { > DeviceState *dev; > > @@ -263,9 +263,12 @@ DeviceState *pxa2xx_gpio_init(target_phys_addr_t base, > qdev_init_nofail(dev); > > sysbus_mmio_map(sysbus_from_qdev(dev), 0, base); > - sysbus_connect_irq(sysbus_from_qdev(dev), 0, pic[PXA2XX_PIC_GPIO_0]); > - sysbus_connect_irq(sysbus_from_qdev(dev), 1, pic[PXA2XX_PIC_GPIO_1]); > - sysbus_connect_irq(sysbus_from_qdev(dev), 2, pic[PXA2XX_PIC_GPIO_X]); > + sysbus_connect_irq(sysbus_from_qdev(dev), 0, > + qdev_get_gpio_in(pic, PXA2XX_PIC_GPIO_0)); > + sysbus_connect_irq(sysbus_from_qdev(dev), 1, > + qdev_get_gpio_in(pic, PXA2XX_PIC_GPIO_1)); > + sysbus_connect_irq(sysbus_from_qdev(dev), 2, > + qdev_get_gpio_in(pic, PXA2XX_PIC_GPIO_X)); > > return dev; > } > diff --git a/hw/pxa2xx_pic.c b/hw/pxa2xx_pic.c > index a36da23..1053b92 100644 > --- a/hw/pxa2xx_pic.c > +++ b/hw/pxa2xx_pic.c > @@ -10,6 +10,8 @@ > > #include "hw.h" > #include "pxa.h" > +#include "arm-misc.h" > +#include "sysbus.h" > > #define ICIP 0x00 /* Interrupt Controller IRQ Pending register */ > #define ICMR 0x04 /* Interrupt Controller Mask register */ > @@ -31,7 +33,10 @@ > #define PXA2XX_PIC_SRCS 40 > > typedef struct { > - CPUState *cpu_env; > + SysBusDevice busdev; > + qemu_irq hard_irq; > + qemu_irq fiq_irq; > + qemu_irq wake_irq; > uint32_t int_enabled[2]; > uint32_t int_pending[2]; > uint32_t is_fiq[2]; > @@ -44,25 +49,18 @@ static void pxa2xx_pic_update(void *opaque) > uint32_t mask[2]; > PXA2xxPICState *s = (PXA2xxPICState *) opaque; > > - if (s->cpu_env->halted) { > - mask[0] = s->int_pending[0] & (s->int_enabled[0] | s->int_idle); > - mask[1] = s->int_pending[1] & (s->int_enabled[1] | s->int_idle); > - if (mask[0] || mask[1]) > - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_EXITTB); > - } > + mask[0] = s->int_pending[0] & (s->int_enabled[0] | s->int_idle); > + mask[1] = s->int_pending[1] & (s->int_enabled[1] | s->int_idle); > + qemu_set_irq(s->wake_irq, mask[0] || mask[1]); > > mask[0] = s->int_pending[0] & s->int_enabled[0]; > mask[1] = s->int_pending[1] & s->int_enabled[1]; > > - if ((mask[0] & s->is_fiq[0]) || (mask[1] & s->is_fiq[1])) > - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_FIQ); > - else > - cpu_reset_interrupt(s->cpu_env, CPU_INTERRUPT_FIQ); > + qemu_set_irq(s->fiq_irq, > + ((mask[0] & s->is_fiq[0]) || (mask[1] & s->is_fiq[1]))); > > - if ((mask[0] & ~s->is_fiq[0]) || (mask[1] & ~s->is_fiq[1])) > - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); > - else > - cpu_reset_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); > + qemu_set_irq(s->hard_irq, > + ((mask[0] & ~s->is_fiq[0]) || (mask[1] & ~s->is_fiq[1]))); > } > > /* Note: Here level means state of the signal on a pin, not > @@ -241,53 +239,33 @@ static CPUWriteMemoryFunc * const pxa2xx_pic_writefn[] = { > pxa2xx_pic_mem_write, > }; > > -static void pxa2xx_pic_save(QEMUFile *f, void *opaque) > +static int pxa2xx_pic_post_load(void *opaque, int version_id) > { > - PXA2xxPICState *s = (PXA2xxPICState *) opaque; > - int i; > - > - for (i = 0; i < 2; i ++) > - qemu_put_be32s(f, &s->int_enabled[i]); > - for (i = 0; i < 2; i ++) > - qemu_put_be32s(f, &s->int_pending[i]); > - for (i = 0; i < 2; i ++) > - qemu_put_be32s(f, &s->is_fiq[i]); > - qemu_put_be32s(f, &s->int_idle); > - for (i = 0; i < PXA2XX_PIC_SRCS; i ++) > - qemu_put_be32s(f, &s->priority[i]); > + pxa2xx_pic_update(opaque); > + return 0; > } > > -static int pxa2xx_pic_load(QEMUFile *f, void *opaque, int version_id) > +DeviceState *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env, > + qemu_irq *arm_pic) > { > - PXA2xxPICState *s = (PXA2xxPICState *) opaque; > - int i; > - > - for (i = 0; i < 2; i ++) > - qemu_get_be32s(f, &s->int_enabled[i]); > - for (i = 0; i < 2; i ++) > - qemu_get_be32s(f, &s->int_pending[i]); > - for (i = 0; i < 2; i ++) > - qemu_get_be32s(f, &s->is_fiq[i]); > - qemu_get_be32s(f, &s->int_idle); > - for (i = 0; i < PXA2XX_PIC_SRCS; i ++) > - qemu_get_be32s(f, &s->priority[i]); > + DeviceState *dev; > > - pxa2xx_pic_update(opaque); > - return 0; > + dev = sysbus_create_varargs("pxa2xx_pic", base, > + arm_pic[ARM_PIC_CPU_IRQ], > + arm_pic[ARM_PIC_CPU_FIQ], > + arm_pic[ARM_PIC_CPU_WAKE], > + NULL); > + > + /* Enable IC coprocessor access. */ > + cpu_arm_set_cp_io(env, 6, pxa2xx_pic_cp_read, pxa2xx_pic_cp_write, dev); I changed the last parameter to "s" as passing dev here was hacky. Cheers
On 2/25/11, andrzej zaborowski <balrogg@gmail.com> wrote: > Hi Dmitry, > > On 20 February 2011 14:50, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> > wrote: >> Use qdev/sysbus framework to handle pxa2xx-pic. Instead of exposing IRQs >> via array, reference them via qdev_get_gpio_in(). Also pxa2xx_pic >> duplicated >> some code from arm-pic. Drop it, replacing with references to arm-pic, >> as all other ARM SoCs do for their PIC code. > > As I said earlier not using arm-pic was deliberate (and I also asked > what the gain was from converting the pic to a separate sysbus device > from the CPU) so I skipped this part of the patch and pushed the rest > of it, please check that everything works. The primary goal was using arm-pic IRQs in pxa2xx-gpio and not having to mess with passing CPUEnv around. Moreover all other ARM SoCs use arm-pic w/o any references to performance gains/loses. I can still provide a patch that will use arm-pic only for pxa2xx-gpio, will that be suitable for you? BTW: it seems that your version won't work: using of sysbus_init_mmio() is hackish and there is no place where that mmio region will be mapped to base. About mapping pic to a separate device from CPU. Initially I wanted to reuse somehow pxa2xx-pic for sa-11[0-1]0 emulation. It doesn't seem reasonable for me anymore anyway. Second, the pic is already in separate module, so I didn't want to disturb main pxa2xx.c with it. I might still later use pxa2xx-pic for allocating main CPU structure and making all other device hang on ot. >> diff --git a/hw/mainstone.c b/hw/mainstone.c >> index aec8d34..4eabdb9 100644 >> --- a/hw/mainstone.c >> +++ b/hw/mainstone.c >> @@ -140,7 +140,7 @@ static void mainstone_common_init(ram_addr_t ram_size, >> } >> >> mst_irq = sysbus_create_simple("mainstone-fpga", MST_FPGA_PHYS, >> - cpu->pic[PXA2XX_PIC_GPIO_0]); >> + qdev_get_gpio_in(cpu->pic, PXA2XX_PIC_GPIO_0)); > > I'm also wondering if this device should really use the interrupt line > instead of using a GPIO. It seems wrong that both the fpga and the > gpio module are connected to the same line. Fixed, will submit a fix soon. >> @@ -241,53 +239,33 @@ static CPUWriteMemoryFunc * const >> pxa2xx_pic_writefn[] = { >> pxa2xx_pic_mem_write, >> }; >> >> -static void pxa2xx_pic_save(QEMUFile *f, void *opaque) >> +static int pxa2xx_pic_post_load(void *opaque, int version_id) >> { >> - PXA2xxPICState *s = (PXA2xxPICState *) opaque; >> - int i; >> - >> - for (i = 0; i < 2; i ++) >> - qemu_put_be32s(f, &s->int_enabled[i]); >> - for (i = 0; i < 2; i ++) >> - qemu_put_be32s(f, &s->int_pending[i]); >> - for (i = 0; i < 2; i ++) >> - qemu_put_be32s(f, &s->is_fiq[i]); >> - qemu_put_be32s(f, &s->int_idle); >> - for (i = 0; i < PXA2XX_PIC_SRCS; i ++) >> - qemu_put_be32s(f, &s->priority[i]); >> + pxa2xx_pic_update(opaque); >> + return 0; >> } >> >> -static int pxa2xx_pic_load(QEMUFile *f, void *opaque, int version_id) >> +DeviceState *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env, >> + qemu_irq *arm_pic) >> { >> - PXA2xxPICState *s = (PXA2xxPICState *) opaque; >> - int i; >> - >> - for (i = 0; i < 2; i ++) >> - qemu_get_be32s(f, &s->int_enabled[i]); >> - for (i = 0; i < 2; i ++) >> - qemu_get_be32s(f, &s->int_pending[i]); >> - for (i = 0; i < 2; i ++) >> - qemu_get_be32s(f, &s->is_fiq[i]); >> - qemu_get_be32s(f, &s->int_idle); >> - for (i = 0; i < PXA2XX_PIC_SRCS; i ++) >> - qemu_get_be32s(f, &s->priority[i]); >> + DeviceState *dev; >> >> - pxa2xx_pic_update(opaque); >> - return 0; >> + dev = sysbus_create_varargs("pxa2xx_pic", base, >> + arm_pic[ARM_PIC_CPU_IRQ], >> + arm_pic[ARM_PIC_CPU_FIQ], >> + arm_pic[ARM_PIC_CPU_WAKE], >> + NULL); >> + >> + /* Enable IC coprocessor access. */ >> + cpu_arm_set_cp_io(env, 6, pxa2xx_pic_cp_read, pxa2xx_pic_cp_write, >> dev); > > I changed the last parameter to "s" as passing dev here was hacky. Fine with me. BTW: what about all other patches?
On 25 February 2011 14:50, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote: > On 2/25/11, andrzej zaborowski <balrogg@gmail.com> wrote: >> Hi Dmitry, >> >> On 20 February 2011 14:50, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> >> wrote: >>> Use qdev/sysbus framework to handle pxa2xx-pic. Instead of exposing IRQs >>> via array, reference them via qdev_get_gpio_in(). Also pxa2xx_pic >>> duplicated >>> some code from arm-pic. Drop it, replacing with references to arm-pic, >>> as all other ARM SoCs do for their PIC code. >> >> As I said earlier not using arm-pic was deliberate (and I also asked >> what the gain was from converting the pic to a separate sysbus device >> from the CPU) so I skipped this part of the patch and pushed the rest >> of it, please check that everything works. > > The primary goal was using arm-pic IRQs in pxa2xx-gpio and not having to > mess with passing CPUEnv around. Moreover all other ARM SoCs use > arm-pic w/o any references to performance gains/loses. > > I can still provide a patch that will use arm-pic only for > pxa2xx-gpio, will that > be suitable for you? Well my take on it is that it adds an additional level indirection. More levels of indirection / abstraction are only good if they enable some new feature or make a big difference in the code and it's not the case here. Otherwise they make bloat and more levels of function calls which add up to small overheads. And I don't see using CPUEnv as a hack anyway. I pushed the remaining patches with small adjustments. Cheers
diff --git a/hw/mainstone.c b/hw/mainstone.c index aec8d34..4eabdb9 100644 --- a/hw/mainstone.c +++ b/hw/mainstone.c @@ -140,7 +140,7 @@ static void mainstone_common_init(ram_addr_t ram_size, } mst_irq = sysbus_create_simple("mainstone-fpga", MST_FPGA_PHYS, - cpu->pic[PXA2XX_PIC_GPIO_0]); + qdev_get_gpio_in(cpu->pic, PXA2XX_PIC_GPIO_0)); /* setup keypad */ printf("map addr %p\n", &map); diff --git a/hw/pxa.h b/hw/pxa.h index f73d33b..7c6fd44 100644 --- a/hw/pxa.h +++ b/hw/pxa.h @@ -63,15 +63,16 @@ # define PXA2XX_INTERNAL_SIZE 0x40000 /* pxa2xx_pic.c */ -qemu_irq *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env); +DeviceState *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env, + qemu_irq *arm_pic); /* pxa2xx_timer.c */ -void pxa25x_timer_init(target_phys_addr_t base, qemu_irq *irqs); -void pxa27x_timer_init(target_phys_addr_t base, qemu_irq *irqs, qemu_irq irq4); +void pxa25x_timer_init(target_phys_addr_t base, DeviceState *pic); +void pxa27x_timer_init(target_phys_addr_t base, DeviceState *pic); /* pxa2xx_gpio.c */ DeviceState *pxa2xx_gpio_init(target_phys_addr_t base, - CPUState *env, qemu_irq *pic, int lines); + CPUState *env, DeviceState *pic, int lines); void pxa2xx_gpio_read_notifier(DeviceState *dev, qemu_irq handler); /* pxa2xx_dma.c */ @@ -125,7 +126,7 @@ typedef struct PXA2xxFIrState PXA2xxFIrState; typedef struct { CPUState *env; - qemu_irq *pic; + DeviceState *pic; qemu_irq reset; PXA2xxDMAState *dma; DeviceState *gpio; @@ -180,6 +181,7 @@ typedef struct { QEMUTimer *rtc_swal1; QEMUTimer *rtc_swal2; QEMUTimer *rtc_pi; + qemu_irq rtc_irq; } PXA2xxState; struct PXA2xxI2SState { diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c index 9ebbce6..58e6e7b 100644 --- a/hw/pxa2xx.c +++ b/hw/pxa2xx.c @@ -16,6 +16,7 @@ #include "qemu-timer.h" #include "qemu-char.h" #include "blockdev.h" +#include "arm-misc.h" static struct { target_phys_addr_t io_base; @@ -888,7 +889,7 @@ static int pxa2xx_ssp_init(SysBusDevice *dev) static inline void pxa2xx_rtc_int_update(PXA2xxState *s) { - qemu_set_irq(s->pic[PXA2XX_PIC_RTCALARM], !!(s->rtsr & 0x2553)); + qemu_set_irq(s->rtc_irq, !!(s->rtsr & 0x2553)); } static void pxa2xx_rtc_hzupdate(PXA2xxState *s) @@ -1197,6 +1198,8 @@ static void pxa2xx_rtc_init(PXA2xxState *s) s->rtc_swal1 = qemu_new_timer(rt_clock, pxa2xx_rtc_swal1_tick, s); s->rtc_swal2 = qemu_new_timer(rt_clock, pxa2xx_rtc_swal2_tick, s); s->rtc_pi = qemu_new_timer(rt_clock, pxa2xx_rtc_pi_tick, s); + + s->rtc_irq = qdev_get_gpio_in(s->pic, PXA2XX_PIC_RTCALARM); } static void pxa2xx_rtc_save(QEMUFile *f, void *opaque) @@ -2069,6 +2072,8 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) PXA2xxState *s; int iomemtype, i; DriveInfo *dinfo; + qemu_irq *arm_pic; + s = (PXA2xxState *) qemu_mallocz(sizeof(PXA2xxState)); if (revision && strncmp(revision, "pxa27", 5)) { @@ -2093,12 +2098,13 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) 0x40000, qemu_ram_alloc(NULL, "pxa270.internal", 0x40000) | IO_MEM_RAM); - s->pic = pxa2xx_pic_init(0x40d00000, s->env); + arm_pic = arm_pic_init_cpu(s->env); + s->pic = pxa2xx_pic_init(0x40d00000, s->env, arm_pic); - s->dma = pxa27x_dma_init(0x40000000, s->pic[PXA2XX_PIC_DMA]); + s->dma = pxa27x_dma_init(0x40000000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_DMA)); - pxa27x_timer_init(0x40a00000, &s->pic[PXA2XX_PIC_OST_0], - s->pic[PXA27X_PIC_OST_4_11]); + pxa27x_timer_init(0x40a00000, s->pic); s->gpio = pxa2xx_gpio_init(0x40e00000, s->env, s->pic, 121); @@ -2108,26 +2114,30 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) exit(1); } s->mmc = pxa2xx_mmci_init(0x41100000, dinfo->bdrv, - s->pic[PXA2XX_PIC_MMC], s->dma); + qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC), s->dma); for (i = 0; pxa270_serial[i].io_base; i ++) if (serial_hds[i]) #ifdef TARGET_WORDS_BIGENDIAN serial_mm_init(pxa270_serial[i].io_base, 2, - s->pic[pxa270_serial[i].irqn], 14857000/16, + qdev_get_gpio_in(s->pic, pxa270_serial[i].irqn), + 14857000/16, serial_hds[i], 1, 1); #else serial_mm_init(pxa270_serial[i].io_base, 2, - s->pic[pxa270_serial[i].irqn], 14857000/16, + qdev_get_gpio_in(s->pic, pxa270_serial[i].irqn), + 14857000/16, serial_hds[i], 1, 0); #endif else break; if (serial_hds[i]) - s->fir = pxa2xx_fir_init(0x40800000, s->pic[PXA2XX_PIC_ICP], + s->fir = pxa2xx_fir_init(0x40800000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_ICP), s->dma, serial_hds[i]); - s->lcd = pxa2xx_lcdc_init(0x44000000, s->pic[PXA2XX_PIC_LCD]); + s->lcd = pxa2xx_lcdc_init(0x44000000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_LCD)); s->cm_base = 0x41300000; s->cm_regs[CCCR >> 2] = 0x02000210; /* 416.0 MHz */ @@ -2159,13 +2169,13 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) for (i = 0; pxa27x_ssp[i].io_base; i ++) { DeviceState *dev; dev = sysbus_create_simple("pxa2xx-ssp", pxa27x_ssp[i].io_base, - s->pic[pxa27x_ssp[i].irqn]); + qdev_get_gpio_in(s->pic, pxa27x_ssp[i].irqn)); s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); } if (usb_enabled) { sysbus_create_simple("sysbus-ohci", 0x4c000000, - s->pic[PXA2XX_PIC_USBH1]); + qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); } s->pcmcia[0] = pxa2xx_pcmcia_init(0x20000000); @@ -2179,12 +2189,17 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) register_savevm(NULL, "pxa2xx_rtc", 0, 0, pxa2xx_rtc_save, pxa2xx_rtc_load, s); - s->i2c[0] = pxa2xx_i2c_init(0x40301600, s->pic[PXA2XX_PIC_I2C], 0xffff); - s->i2c[1] = pxa2xx_i2c_init(0x40f00100, s->pic[PXA2XX_PIC_PWRI2C], 0xff); + s->i2c[0] = pxa2xx_i2c_init(0x40301600, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_I2C), 0xffff); + s->i2c[1] = pxa2xx_i2c_init(0x40f00100, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_PWRI2C), 0xff); - s->i2s = pxa2xx_i2s_init(0x40400000, s->pic[PXA2XX_PIC_I2S], s->dma); + s->i2s = pxa2xx_i2s_init(0x40400000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_I2S), + s->dma); - s->kp = pxa27x_keypad_init(0x41500000, s->pic[PXA2XX_PIC_KEYPAD]); + s->kp = pxa27x_keypad_init(0x41500000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_KEYPAD)); /* GPIO1 resets the processor */ /* The handler can be overridden by board-specific code */ @@ -2198,6 +2213,7 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) PXA2xxState *s; int iomemtype, i; DriveInfo *dinfo; + qemu_irq *arm_pic; s = (PXA2xxState *) qemu_mallocz(sizeof(PXA2xxState)); @@ -2216,11 +2232,13 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) qemu_ram_alloc(NULL, "pxa255.internal", PXA2XX_INTERNAL_SIZE) | IO_MEM_RAM); - s->pic = pxa2xx_pic_init(0x40d00000, s->env); + arm_pic = arm_pic_init_cpu(s->env); + s->pic = pxa2xx_pic_init(0x40d00000, s->env, arm_pic); - s->dma = pxa255_dma_init(0x40000000, s->pic[PXA2XX_PIC_DMA]); + s->dma = pxa255_dma_init(0x40000000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_DMA)); - pxa25x_timer_init(0x40a00000, &s->pic[PXA2XX_PIC_OST_0]); + pxa25x_timer_init(0x40a00000, s->pic); s->gpio = pxa2xx_gpio_init(0x40e00000, s->env, s->pic, 85); @@ -2230,27 +2248,31 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) exit(1); } s->mmc = pxa2xx_mmci_init(0x41100000, dinfo->bdrv, - s->pic[PXA2XX_PIC_MMC], s->dma); + qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC), s->dma); for (i = 0; pxa255_serial[i].io_base; i ++) if (serial_hds[i]) { #ifdef TARGET_WORDS_BIGENDIAN serial_mm_init(pxa255_serial[i].io_base, 2, - s->pic[pxa255_serial[i].irqn], 14745600/16, + qdev_get_gpio_in(s->pic, pxa255_serial[i].irqn), + 14745600/16, serial_hds[i], 1, 1); #else serial_mm_init(pxa255_serial[i].io_base, 2, - s->pic[pxa255_serial[i].irqn], 14745600/16, + qdev_get_gpio_in(s->pic, pxa255_serial[i].irqn), + 14745600/16, serial_hds[i], 1, 0); #endif } else { break; } if (serial_hds[i]) - s->fir = pxa2xx_fir_init(0x40800000, s->pic[PXA2XX_PIC_ICP], + s->fir = pxa2xx_fir_init(0x40800000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_ICP), s->dma, serial_hds[i]); - s->lcd = pxa2xx_lcdc_init(0x44000000, s->pic[PXA2XX_PIC_LCD]); + s->lcd = pxa2xx_lcdc_init(0x44000000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_LCD)); s->cm_base = 0x41300000; s->cm_regs[CCCR >> 2] = 0x02000210; /* 416.0 MHz */ @@ -2282,13 +2304,13 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) for (i = 0; pxa255_ssp[i].io_base; i ++) { DeviceState *dev; dev = sysbus_create_simple("pxa2xx-ssp", pxa255_ssp[i].io_base, - s->pic[pxa255_ssp[i].irqn]); + qdev_get_gpio_in(s->pic, pxa255_ssp[i].irqn)); s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); } if (usb_enabled) { sysbus_create_simple("sysbus-ohci", 0x4c000000, - s->pic[PXA2XX_PIC_USBH1]); + qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); } s->pcmcia[0] = pxa2xx_pcmcia_init(0x20000000); @@ -2302,10 +2324,14 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) register_savevm(NULL, "pxa2xx_rtc", 0, 0, pxa2xx_rtc_save, pxa2xx_rtc_load, s); - s->i2c[0] = pxa2xx_i2c_init(0x40301600, s->pic[PXA2XX_PIC_I2C], 0xffff); - s->i2c[1] = pxa2xx_i2c_init(0x40f00100, s->pic[PXA2XX_PIC_PWRI2C], 0xff); + s->i2c[0] = pxa2xx_i2c_init(0x40301600, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_I2C), 0xffff); + s->i2c[1] = pxa2xx_i2c_init(0x40f00100, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_PWRI2C), 0xff); - s->i2s = pxa2xx_i2s_init(0x40400000, s->pic[PXA2XX_PIC_I2S], s->dma); + s->i2s = pxa2xx_i2s_init(0x40400000, + qdev_get_gpio_in(s->pic, PXA2XX_PIC_I2S), + s->dma); /* GPIO1 resets the processor */ /* The handler can be overridden by board-specific code */ diff --git a/hw/pxa2xx_gpio.c b/hw/pxa2xx_gpio.c index 789965d..16a8865 100644 --- a/hw/pxa2xx_gpio.c +++ b/hw/pxa2xx_gpio.c @@ -253,7 +253,7 @@ static CPUWriteMemoryFunc * const pxa2xx_gpio_writefn[] = { }; DeviceState *pxa2xx_gpio_init(target_phys_addr_t base, - CPUState *env, qemu_irq *pic, int lines) + CPUState *env, DeviceState *pic, int lines) { DeviceState *dev; @@ -263,9 +263,12 @@ DeviceState *pxa2xx_gpio_init(target_phys_addr_t base, qdev_init_nofail(dev); sysbus_mmio_map(sysbus_from_qdev(dev), 0, base); - sysbus_connect_irq(sysbus_from_qdev(dev), 0, pic[PXA2XX_PIC_GPIO_0]); - sysbus_connect_irq(sysbus_from_qdev(dev), 1, pic[PXA2XX_PIC_GPIO_1]); - sysbus_connect_irq(sysbus_from_qdev(dev), 2, pic[PXA2XX_PIC_GPIO_X]); + sysbus_connect_irq(sysbus_from_qdev(dev), 0, + qdev_get_gpio_in(pic, PXA2XX_PIC_GPIO_0)); + sysbus_connect_irq(sysbus_from_qdev(dev), 1, + qdev_get_gpio_in(pic, PXA2XX_PIC_GPIO_1)); + sysbus_connect_irq(sysbus_from_qdev(dev), 2, + qdev_get_gpio_in(pic, PXA2XX_PIC_GPIO_X)); return dev; } diff --git a/hw/pxa2xx_pic.c b/hw/pxa2xx_pic.c index a36da23..1053b92 100644 --- a/hw/pxa2xx_pic.c +++ b/hw/pxa2xx_pic.c @@ -10,6 +10,8 @@ #include "hw.h" #include "pxa.h" +#include "arm-misc.h" +#include "sysbus.h" #define ICIP 0x00 /* Interrupt Controller IRQ Pending register */ #define ICMR 0x04 /* Interrupt Controller Mask register */ @@ -31,7 +33,10 @@ #define PXA2XX_PIC_SRCS 40 typedef struct { - CPUState *cpu_env; + SysBusDevice busdev; + qemu_irq hard_irq; + qemu_irq fiq_irq; + qemu_irq wake_irq; uint32_t int_enabled[2]; uint32_t int_pending[2]; uint32_t is_fiq[2]; @@ -44,25 +49,18 @@ static void pxa2xx_pic_update(void *opaque) uint32_t mask[2]; PXA2xxPICState *s = (PXA2xxPICState *) opaque; - if (s->cpu_env->halted) { - mask[0] = s->int_pending[0] & (s->int_enabled[0] | s->int_idle); - mask[1] = s->int_pending[1] & (s->int_enabled[1] | s->int_idle); - if (mask[0] || mask[1]) - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_EXITTB); - } + mask[0] = s->int_pending[0] & (s->int_enabled[0] | s->int_idle); + mask[1] = s->int_pending[1] & (s->int_enabled[1] | s->int_idle); + qemu_set_irq(s->wake_irq, mask[0] || mask[1]); mask[0] = s->int_pending[0] & s->int_enabled[0]; mask[1] = s->int_pending[1] & s->int_enabled[1]; - if ((mask[0] & s->is_fiq[0]) || (mask[1] & s->is_fiq[1])) - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_FIQ); - else - cpu_reset_interrupt(s->cpu_env, CPU_INTERRUPT_FIQ); + qemu_set_irq(s->fiq_irq, + ((mask[0] & s->is_fiq[0]) || (mask[1] & s->is_fiq[1]))); - if ((mask[0] & ~s->is_fiq[0]) || (mask[1] & ~s->is_fiq[1])) - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); - else - cpu_reset_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); + qemu_set_irq(s->hard_irq, + ((mask[0] & ~s->is_fiq[0]) || (mask[1] & ~s->is_fiq[1]))); } /* Note: Here level means state of the signal on a pin, not @@ -241,53 +239,33 @@ static CPUWriteMemoryFunc * const pxa2xx_pic_writefn[] = { pxa2xx_pic_mem_write, }; -static void pxa2xx_pic_save(QEMUFile *f, void *opaque) +static int pxa2xx_pic_post_load(void *opaque, int version_id) { - PXA2xxPICState *s = (PXA2xxPICState *) opaque; - int i; - - for (i = 0; i < 2; i ++) - qemu_put_be32s(f, &s->int_enabled[i]); - for (i = 0; i < 2; i ++) - qemu_put_be32s(f, &s->int_pending[i]); - for (i = 0; i < 2; i ++) - qemu_put_be32s(f, &s->is_fiq[i]); - qemu_put_be32s(f, &s->int_idle); - for (i = 0; i < PXA2XX_PIC_SRCS; i ++) - qemu_put_be32s(f, &s->priority[i]); + pxa2xx_pic_update(opaque); + return 0; } -static int pxa2xx_pic_load(QEMUFile *f, void *opaque, int version_id) +DeviceState *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env, + qemu_irq *arm_pic) { - PXA2xxPICState *s = (PXA2xxPICState *) opaque; - int i; - - for (i = 0; i < 2; i ++) - qemu_get_be32s(f, &s->int_enabled[i]); - for (i = 0; i < 2; i ++) - qemu_get_be32s(f, &s->int_pending[i]); - for (i = 0; i < 2; i ++) - qemu_get_be32s(f, &s->is_fiq[i]); - qemu_get_be32s(f, &s->int_idle); - for (i = 0; i < PXA2XX_PIC_SRCS; i ++) - qemu_get_be32s(f, &s->priority[i]); + DeviceState *dev; - pxa2xx_pic_update(opaque); - return 0; + dev = sysbus_create_varargs("pxa2xx_pic", base, + arm_pic[ARM_PIC_CPU_IRQ], + arm_pic[ARM_PIC_CPU_FIQ], + arm_pic[ARM_PIC_CPU_WAKE], + NULL); + + /* Enable IC coprocessor access. */ + cpu_arm_set_cp_io(env, 6, pxa2xx_pic_cp_read, pxa2xx_pic_cp_write, dev); + + return dev; } -qemu_irq *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env) +static int pxa2xx_pic_initfn(SysBusDevice *dev) { - PXA2xxPICState *s; + PXA2xxPICState *s = FROM_SYSBUS(PXA2xxPICState, dev); int iomemtype; - qemu_irq *qi; - - s = (PXA2xxPICState *) - qemu_mallocz(sizeof(PXA2xxPICState)); - if (!s) - return NULL; - - s->cpu_env = env; s->int_pending[0] = 0; s->int_pending[1] = 0; @@ -296,18 +274,46 @@ qemu_irq *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env) s->is_fiq[0] = 0; s->is_fiq[1] = 0; - qi = qemu_allocate_irqs(pxa2xx_pic_set_irq, s, PXA2XX_PIC_SRCS); + sysbus_init_irq(dev, &s->hard_irq); + sysbus_init_irq(dev, &s->fiq_irq); + sysbus_init_irq(dev, &s->wake_irq); + + qdev_init_gpio_in(&dev->qdev, pxa2xx_pic_set_irq, PXA2XX_PIC_SRCS); /* Enable IC memory-mapped registers access. */ iomemtype = cpu_register_io_memory(pxa2xx_pic_readfn, pxa2xx_pic_writefn, s, DEVICE_NATIVE_ENDIAN); - cpu_register_physical_memory(base, 0x00100000, iomemtype); + sysbus_init_mmio(dev, 0x00100000, iomemtype); - /* Enable IC coprocessor access. */ - cpu_arm_set_cp_io(env, 6, pxa2xx_pic_cp_read, pxa2xx_pic_cp_write, s); + return 0; +} - register_savevm(NULL, "pxa2xx_pic", 0, 0, pxa2xx_pic_save, - pxa2xx_pic_load, s); +static VMStateDescription vmstate_pxa2xx_pic_regs = { + .name = "pxa2xx_pic", + .version_id = 0, + .minimum_version_id = 0, + .minimum_version_id_old = 0, + .post_load = pxa2xx_pic_post_load, + .fields = (VMStateField[]) { + VMSTATE_UINT32_ARRAY(int_enabled, PXA2xxPICState, 2), + VMSTATE_UINT32_ARRAY(int_pending, PXA2xxPICState, 2), + VMSTATE_UINT32_ARRAY(is_fiq, PXA2xxPICState, 2), + VMSTATE_UINT32(int_idle, PXA2xxPICState), + VMSTATE_UINT32_ARRAY(priority, PXA2xxPICState, PXA2XX_PIC_SRCS), + VMSTATE_END_OF_LIST(), + }, +}; - return qi; +static SysBusDeviceInfo pxa2xx_pic_info = { + .init = pxa2xx_pic_initfn, + .qdev.name = "pxa2xx_pic", + .qdev.desc = "PXA2xx PIC", + .qdev.size = sizeof(PXA2xxPICState), + .qdev.vmsd = &vmstate_pxa2xx_pic_regs, +}; + +static void pxa2xx_pic_register(void) +{ + sysbus_register_withprop(&pxa2xx_pic_info); } +device_init(pxa2xx_pic_register); diff --git a/hw/pxa2xx_timer.c b/hw/pxa2xx_timer.c index b556d11..7ab2cc3 100644 --- a/hw/pxa2xx_timer.c +++ b/hw/pxa2xx_timer.c @@ -10,6 +10,7 @@ #include "hw.h" #include "qemu-timer.h" #include "sysemu.h" +#include "qdev.h" #include "pxa.h" #define OSMR0 0x00 @@ -428,7 +429,7 @@ static int pxa2xx_timer_load(QEMUFile *f, void *opaque, int version_id) } static pxa2xx_timer_info *pxa2xx_timer_init(target_phys_addr_t base, - qemu_irq *irqs) + DeviceState *pic) { int i; int iomemtype; @@ -443,7 +444,7 @@ static pxa2xx_timer_info *pxa2xx_timer_init(target_phys_addr_t base, for (i = 0; i < 4; i ++) { s->timer[i].value = 0; - s->timer[i].irq = irqs[i]; + s->timer[i].irq = qdev_get_gpio_in(pic, PXA2XX_PIC_OST_0 + i); s->timer[i].info = s; s->timer[i].num = i; s->timer[i].level = 0; @@ -461,24 +462,23 @@ static pxa2xx_timer_info *pxa2xx_timer_init(target_phys_addr_t base, return s; } -void pxa25x_timer_init(target_phys_addr_t base, qemu_irq *irqs) +void pxa25x_timer_init(target_phys_addr_t base, DeviceState *pic) { - pxa2xx_timer_info *s = pxa2xx_timer_init(base, irqs); + pxa2xx_timer_info *s = pxa2xx_timer_init(base, pic); s->freq = PXA25X_FREQ; s->tm4 = NULL; } -void pxa27x_timer_init(target_phys_addr_t base, - qemu_irq *irqs, qemu_irq irq4) +void pxa27x_timer_init(target_phys_addr_t base, DeviceState *pic) { - pxa2xx_timer_info *s = pxa2xx_timer_init(base, irqs); + pxa2xx_timer_info *s = pxa2xx_timer_init(base, pic); int i; s->freq = PXA27X_FREQ; s->tm4 = (PXA2xxTimer4 *) qemu_mallocz(8 * sizeof(PXA2xxTimer4)); for (i = 0; i < 8; i ++) { s->tm4[i].tm.value = 0; - s->tm4[i].tm.irq = irq4; + s->tm4[i].tm.irq = qdev_get_gpio_in(pic, PXA27X_PIC_OST_4_11); s->tm4[i].tm.info = s; s->tm4[i].tm.num = i + 4; s->tm4[i].tm.level = 0;
Use qdev/sysbus framework to handle pxa2xx-pic. Instead of exposing IRQs via array, reference them via qdev_get_gpio_in(). Also pxa2xx_pic duplicated some code from arm-pic. Drop it, replacing with references to arm-pic, as all other ARM SoCs do for their PIC code. Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> --- hw/mainstone.c | 2 +- hw/pxa.h | 12 +++-- hw/pxa2xx.c | 84 +++++++++++++++++++++++------------ hw/pxa2xx_gpio.c | 11 +++-- hw/pxa2xx_pic.c | 126 ++++++++++++++++++++++++++++------------------------- hw/pxa2xx_timer.c | 16 +++--- 6 files changed, 144 insertions(+), 107 deletions(-)