[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] hw/omap_gpio.c: Convert to qdev
|
From: |
andrzej zaborowski |
|
Subject: |
Re: [Qemu-devel] [PATCH 3/3] hw/omap_gpio.c: Convert to qdev |
|
Date: |
Tue, 5 Jul 2011 00:39:54 +0200 |
Hi,
On 29 June 2011 20:53, Peter Maydell <address@hidden> wrote:
> From: Juha Riihimäki <address@hidden>
>
> Convert the OMAP GPIO module to qdev.
>
> Signed-off-by: Juha Riihimäki <address@hidden>
> [Riku Voipio: Fixes and restructuring patchset]
> Signed-off-by: Riku Voipio <address@hidden>
> [Peter Maydell: More fixes and cleanups for upstream submission]
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> hw/nseries.c | 47 +++++------
> hw/omap.h | 20 +-----
> hw/omap1.c | 10 ++-
> hw/omap2.c | 26 ++++--
> hw/omap_gpio.c | 244 +++++++++++++++++++++++++++----------------------------
> hw/palm.c | 26 +++---
> 6 files changed, 181 insertions(+), 192 deletions(-)
>
> diff --git a/hw/nseries.c b/hw/nseries.c
> index 2f6f473..4ea2d6b 100644
> --- a/hw/nseries.c
> +++ b/hw/nseries.c
> @@ -134,9 +134,9 @@ static void n800_mmc_cs_cb(void *opaque, int line, int
> level)
> static void n8x0_gpio_setup(struct n800_s *s)
> {
> qemu_irq *mmc_cs = qemu_allocate_irqs(n800_mmc_cs_cb, s->cpu->mmc, 1);
> - omap2_gpio_out_set(s->cpu->gpif, N8X0_MMC_CS_GPIO, mmc_cs[0]);
> + qdev_connect_gpio_out(s->cpu->gpio, N8X0_MMC_CS_GPIO, mmc_cs[0]);
>
> - qemu_irq_lower(omap2_gpio_in_get(s->cpu->gpif, N800_BAT_COVER_GPIO)[0]);
> + qemu_irq_lower(qdev_get_gpio_in(s->cpu->gpio, N800_BAT_COVER_GPIO));
> }
>
> #define MAEMO_CAL_HEADER(...) \
> @@ -168,8 +168,8 @@ static void n8x0_nand_setup(struct n800_s *s)
> omap_gpmc_attach(s->cpu->gpmc, N8X0_ONENAND_CS, 0, onenand_base_update,
> onenand_base_unmap,
> (s->nand = onenand_init(0xec4800, 1,
> - omap2_gpio_in_get(s->cpu->gpif,
> - N8X0_ONENAND_GPIO)[0])));
> + qdev_get_gpio_in(s->cpu->gpio,
> + N8X0_ONENAND_GPIO))));
> otp_region = onenand_raw_otp(s->nand);
>
> memcpy(otp_region + 0x000, n8x0_cal_wlan_mac, sizeof(n8x0_cal_wlan_mac));
> @@ -180,7 +180,7 @@ static void n8x0_nand_setup(struct n800_s *s)
> static void n8x0_i2c_setup(struct n800_s *s)
> {
> DeviceState *dev;
> - qemu_irq tmp_irq = omap2_gpio_in_get(s->cpu->gpif, N8X0_TMP105_GPIO)[0];
> + qemu_irq tmp_irq = qdev_get_gpio_in(s->cpu->gpio, N8X0_TMP105_GPIO);
>
> /* Attach the CPU on one end of our I2C bus. */
> s->i2c = omap_i2c_bus(s->cpu->i2c[0]);
> @@ -249,8 +249,8 @@ static void n800_tsc_kbd_setup(struct n800_s *s)
> /* XXX: are the three pins inverted inside the chip between the
> * tsc and the cpu (N4111)? */
> qemu_irq penirq = NULL; /* NC */
> - qemu_irq kbirq = omap2_gpio_in_get(s->cpu->gpif,
> N800_TSC_KP_IRQ_GPIO)[0];
> - qemu_irq dav = omap2_gpio_in_get(s->cpu->gpif, N800_TSC_TS_GPIO)[0];
> + qemu_irq kbirq = qdev_get_gpio_in(s->cpu->gpio, N800_TSC_KP_IRQ_GPIO);
> + qemu_irq dav = qdev_get_gpio_in(s->cpu->gpio, N800_TSC_TS_GPIO);
>
> s->ts.chip = tsc2301_init(penirq, kbirq, dav);
> s->ts.opaque = s->ts.chip->opaque;
> @@ -269,7 +269,7 @@ static void n800_tsc_kbd_setup(struct n800_s *s)
>
> static void n810_tsc_setup(struct n800_s *s)
> {
> - qemu_irq pintdav = omap2_gpio_in_get(s->cpu->gpif, N810_TSC_TS_GPIO)[0];
> + qemu_irq pintdav = qdev_get_gpio_in(s->cpu->gpio, N810_TSC_TS_GPIO);
>
> s->ts.opaque = tsc2005_init(pintdav);
> s->ts.txrx = tsc2005_txrx;
> @@ -361,7 +361,7 @@ static int n810_keys[0x80] = {
>
> static void n810_kbd_setup(struct n800_s *s)
> {
> - qemu_irq kbd_irq = omap2_gpio_in_get(s->cpu->gpif,
> N810_KEYBOARD_GPIO)[0];
> + qemu_irq kbd_irq = qdev_get_gpio_in(s->cpu->gpio, N810_KEYBOARD_GPIO);
> DeviceState *dev;
> int i;
>
> @@ -726,15 +726,15 @@ static void n8x0_dss_setup(struct n800_s *s)
>
> static void n8x0_cbus_setup(struct n800_s *s)
> {
> - qemu_irq dat_out = omap2_gpio_in_get(s->cpu->gpif,
> N8X0_CBUS_DAT_GPIO)[0];
> - qemu_irq retu_irq = omap2_gpio_in_get(s->cpu->gpif, N8X0_RETU_GPIO)[0];
> - qemu_irq tahvo_irq = omap2_gpio_in_get(s->cpu->gpif, N8X0_TAHVO_GPIO)[0];
> + qemu_irq dat_out = qdev_get_gpio_in(s->cpu->gpio, N8X0_CBUS_DAT_GPIO);
> + qemu_irq retu_irq = qdev_get_gpio_in(s->cpu->gpio, N8X0_RETU_GPIO);
> + qemu_irq tahvo_irq = qdev_get_gpio_in(s->cpu->gpio, N8X0_TAHVO_GPIO);
>
> CBus *cbus = cbus_init(dat_out);
>
> - omap2_gpio_out_set(s->cpu->gpif, N8X0_CBUS_CLK_GPIO, cbus->clk);
> - omap2_gpio_out_set(s->cpu->gpif, N8X0_CBUS_DAT_GPIO, cbus->dat);
> - omap2_gpio_out_set(s->cpu->gpif, N8X0_CBUS_SEL_GPIO, cbus->sel);
> + qdev_connect_gpio_out(s->cpu->gpio, N8X0_CBUS_CLK_GPIO, cbus->clk);
> + qdev_connect_gpio_out(s->cpu->gpio, N8X0_CBUS_DAT_GPIO, cbus->dat);
> + qdev_connect_gpio_out(s->cpu->gpio, N8X0_CBUS_SEL_GPIO, cbus->sel);
>
> cbus_attach(cbus, s->retu = retu_init(retu_irq, 1));
> cbus_attach(cbus, s->tahvo = tahvo_init(tahvo_irq, 1));
> @@ -743,13 +743,12 @@ static void n8x0_cbus_setup(struct n800_s *s)
> static void n8x0_uart_setup(struct n800_s *s)
> {
> CharDriverState *radio = uart_hci_init(
> - omap2_gpio_in_get(s->cpu->gpif,
> - N8X0_BT_HOST_WKUP_GPIO)[0]);
> + qdev_get_gpio_in(s->cpu->gpio, N8X0_BT_HOST_WKUP_GPIO));
>
> - omap2_gpio_out_set(s->cpu->gpif, N8X0_BT_RESET_GPIO,
> - csrhci_pins_get(radio)[csrhci_pin_reset]);
> - omap2_gpio_out_set(s->cpu->gpif, N8X0_BT_WKUP_GPIO,
> - csrhci_pins_get(radio)[csrhci_pin_wakeup]);
> + qdev_connect_gpio_out(s->cpu->gpio, N8X0_BT_RESET_GPIO,
> + csrhci_pins_get(radio)[csrhci_pin_reset]);
> + qdev_connect_gpio_out(s->cpu->gpio, N8X0_BT_WKUP_GPIO,
> + csrhci_pins_get(radio)[csrhci_pin_wakeup]);
>
> omap_uart_attach(s->cpu->uart[BT_UART], radio);
> }
> @@ -763,7 +762,7 @@ static void n8x0_usb_power_cb(void *opaque, int line, int
> level)
>
> static void n8x0_usb_setup(struct n800_s *s)
> {
> - qemu_irq tusb_irq = omap2_gpio_in_get(s->cpu->gpif,
> N8X0_TUSB_INT_GPIO)[0];
> + qemu_irq tusb_irq = qdev_get_gpio_in(s->cpu->gpio, N8X0_TUSB_INT_GPIO);
> qemu_irq tusb_pwr = qemu_allocate_irqs(n8x0_usb_power_cb, s, 1)[0];
> TUSBState *tusb = tusb6010_init(tusb_irq);
>
> @@ -774,7 +773,7 @@ static void n8x0_usb_setup(struct n800_s *s)
> tusb6010_sync_io(tusb), NULL, NULL, tusb);
>
> s->usb = tusb;
> - omap2_gpio_out_set(s->cpu->gpif, N8X0_TUSB_ENABLE_GPIO, tusb_pwr);
> + qdev_connect_gpio_out(s->cpu->gpio, N8X0_TUSB_ENABLE_GPIO, tusb_pwr);
> }
>
> /* Setup done before the main bootloader starts by some early setup code
> @@ -1020,7 +1019,7 @@ static void n8x0_boot_init(void *opaque)
>
> /* If the machine has a slided keyboard, open it */
> if (s->kbd)
> - qemu_irq_raise(omap2_gpio_in_get(s->cpu->gpif, N810_SLIDE_GPIO)[0]);
> + qemu_irq_raise(qdev_get_gpio_in(s->cpu->gpio, N810_SLIDE_GPIO));
> }
>
> #define OMAP_TAG_NOKIA_BT 0x4e01
> diff --git a/hw/omap.h b/hw/omap.h
> index 339cc00..2a5b90c 100644
> --- a/hw/omap.h
> +++ b/hw/omap.h
> @@ -682,22 +682,6 @@ qemu_irq *omap_mpuio_in_get(struct omap_mpuio_s *s);
> void omap_mpuio_out_set(struct omap_mpuio_s *s, int line, qemu_irq handler);
> void omap_mpuio_key(struct omap_mpuio_s *s, int row, int col, int down);
>
> -/* omap1 gpio module interface */
> -struct omap_gpio_s;
> -struct omap_gpio_s *omap_gpio_init(target_phys_addr_t base,
> - qemu_irq irq, omap_clk clk);
> -void omap_gpio_reset(struct omap_gpio_s *s);
> -qemu_irq *omap_gpio_in_get(struct omap_gpio_s *s);
> -void omap_gpio_out_set(struct omap_gpio_s *s, int line, qemu_irq handler);
> -
> -/* omap2 gpio interface */
> -struct omap_gpif_s;
> -struct omap_gpif_s *omap2_gpio_init(struct omap_target_agent_s *ta,
> - qemu_irq *irq, omap_clk *fclk, omap_clk iclk, int modules);
> -void omap_gpif_reset(struct omap_gpif_s *s);
> -qemu_irq *omap2_gpio_in_get(struct omap_gpif_s *s, int start);
> -void omap2_gpio_out_set(struct omap_gpif_s *s, int line, qemu_irq handler);
> -
> struct uWireSlave {
> uint16_t (*receive)(void *opaque);
> void (*send)(void *opaque, uint16_t data);
> @@ -851,7 +835,7 @@ struct omap_mpu_state_s {
> /* MPUI-TIPB peripherals */
> struct omap_uart_s *uart[3];
>
> - struct omap_gpio_s *gpio;
> + DeviceState *gpio;
>
> struct omap_mcbsp_s *mcbsp1;
> struct omap_mcbsp_s *mcbsp3;
> @@ -949,8 +933,6 @@ struct omap_mpu_state_s {
> struct omap_gpmc_s *gpmc;
> struct omap_sysctl_s *sysc;
>
> - struct omap_gpif_s *gpif;
> -
> struct omap_mcspi_s *mcspi[2];
>
> struct omap_dss_s *dss;
> diff --git a/hw/omap1.c b/hw/omap1.c
> index 364c26f..625f7cc 100644
> --- a/hw/omap1.c
> +++ b/hw/omap1.c
> @@ -27,6 +27,7 @@
> #include "pc.h"
> #include "blockdev.h"
> #include "range.h"
> +#include "sysbus.h"
>
> /* Should signal the TCMI/GPMC */
> uint32_t omap_badwidth_read8(void *opaque, target_phys_addr_t addr)
> @@ -3585,7 +3586,6 @@ static void omap1_mpu_reset(void *opaque)
> omap_uart_reset(mpu->uart[2]);
> omap_mmc_reset(mpu->mmc);
> omap_mpuio_reset(mpu->mpuio);
> - omap_gpio_reset(mpu->gpio);
> omap_uwire_reset(mpu->microwire);
> omap_pwl_reset(mpu);
> omap_pwt_reset(mpu);
> @@ -3845,8 +3845,12 @@ struct omap_mpu_state_s *omap310_mpu_init(unsigned
> long sdram_size,
> s->irq[1][OMAP_INT_KEYBOARD], s->irq[1][OMAP_INT_MPUIO],
> s->wakeup, omap_findclk(s, "clk32-kHz"));
>
> - s->gpio = omap_gpio_init(0xfffce000, s->irq[0][OMAP_INT_GPIO_BANK1],
> - omap_findclk(s, "arm_gpio_ck"));
> + s->gpio = qdev_create(NULL, "omap_gpio");
> + qdev_prop_set_int32(s->gpio, "mpu_model", s->mpu_model);
> + qdev_init_nofail(s->gpio);
> + sysbus_connect_irq(sysbus_from_qdev(s->gpio), 0,
> + s->irq[0][OMAP_INT_GPIO_BANK1]);
> + sysbus_mmio_map(sysbus_from_qdev(s->gpio), 0, 0xfffce000);
>
> s->microwire = omap_uwire_init(0xfffb3000, &s->irq[1][OMAP_INT_uWireTX],
> s->drq[OMAP_DMA_UWIRE_TX], omap_findclk(s, "mpuper_ck"));
> diff --git a/hw/omap2.c b/hw/omap2.c
> index 0f13272..3b48696 100644
> --- a/hw/omap2.c
> +++ b/hw/omap2.c
> @@ -27,6 +27,7 @@
> #include "qemu-char.h"
> #include "flash.h"
> #include "soc_dma.h"
> +#include "sysbus.h"
> #include "audio/audio.h"
>
> /* Enhanced Audio Controller (CODEC only) */
> @@ -2203,7 +2204,6 @@ static void omap2_mpu_reset(void *opaque)
> omap_uart_reset(mpu->uart[1]);
> omap_uart_reset(mpu->uart[2]);
> omap_mmc_reset(mpu->mmc);
> - omap_gpif_reset(mpu->gpif);
> omap_mcspi_reset(mpu->mcspi[0]);
> omap_mcspi_reset(mpu->mcspi[1]);
> omap_i2c_reset(mpu->i2c[0]);
> @@ -2232,9 +2232,10 @@ struct omap_mpu_state_s *omap2420_mpu_init(unsigned
> long sdram_size,
> ram_addr_t sram_base, q2_base;
> qemu_irq *cpu_irq;
> qemu_irq dma_irqs[4];
> - omap_clk gpio_clks[4];
> DriveInfo *dinfo;
> int i;
> + SysBusDevice *busdev;
> + struct omap_target_agent_s *ta;
>
> /* Core */
> s->mpu_model = omap2420;
> @@ -2377,13 +2378,20 @@ struct omap_mpu_state_s *omap2420_mpu_init(unsigned
> long sdram_size,
> omap_findclk(s, "i2c2.fclk"),
> omap_findclk(s, "i2c2.iclk"));
>
> - gpio_clks[0] = omap_findclk(s, "gpio1_dbclk");
> - gpio_clks[1] = omap_findclk(s, "gpio2_dbclk");
> - gpio_clks[2] = omap_findclk(s, "gpio3_dbclk");
> - gpio_clks[3] = omap_findclk(s, "gpio4_dbclk");
> - s->gpif = omap2_gpio_init(omap_l4ta(s->l4, 3),
> - &s->irq[0][OMAP_INT_24XX_GPIO_BANK1],
> - gpio_clks, omap_findclk(s, "gpio_iclk"), 4);
> + s->gpio = qdev_create(NULL, "omap_gpio");
> + qdev_prop_set_int32(s->gpio, "mpu_model", s->mpu_model);
> + qdev_init_nofail(s->gpio);
> + busdev = sysbus_from_qdev(s->gpio);
> + sysbus_connect_irq(busdev, 0, s->irq[0][OMAP_INT_24XX_GPIO_BANK1]);
> + sysbus_connect_irq(busdev, 3, s->irq[0][OMAP_INT_24XX_GPIO_BANK2]);
> + sysbus_connect_irq(busdev, 6, s->irq[0][OMAP_INT_24XX_GPIO_BANK3]);
> + sysbus_connect_irq(busdev, 9, s->irq[0][OMAP_INT_24XX_GPIO_BANK4]);
> + ta = omap_l4ta(s->l4, 3);
> + sysbus_mmio_map(busdev, 0, omap_l4_base(ta, 1));
> + sysbus_mmio_map(busdev, 1, omap_l4_base(ta, 0));
> + sysbus_mmio_map(busdev, 2, omap_l4_base(ta, 2));
> + sysbus_mmio_map(busdev, 3, omap_l4_base(ta, 4));
> + sysbus_mmio_map(busdev, 4, omap_l4_base(ta, 5));
>
> s->sdrc = omap_sdrc_init(0x68009000);
> s->gpmc = omap_gpmc_init(0x6800a000, s->irq[0][OMAP_INT_24XX_GPMC_IRQ]);
> diff --git a/hw/omap_gpio.c b/hw/omap_gpio.c
> index b53b13b..410d8a6 100644
> --- a/hw/omap_gpio.c
> +++ b/hw/omap_gpio.c
> @@ -20,10 +20,10 @@
>
> #include "hw.h"
> #include "omap.h"
> -/* General-Purpose I/O */
> +#include "sysbus.h"
> +
> struct omap_gpio_s {
> qemu_irq irq;
> - qemu_irq *in;
> qemu_irq handler[16];
>
> uint16_t inputs;
> @@ -35,9 +35,44 @@ struct omap_gpio_s {
> uint16_t pins;
> };
>
> +struct omap2_gpio_s {
> + qemu_irq irq[2];
> + qemu_irq wkup;
> + qemu_irq *handler;
> +
> + uint8_t revision;
> + uint8_t config[2];
> + uint32_t inputs;
> + uint32_t outputs;
> + uint32_t dir;
> + uint32_t level[2];
> + uint32_t edge[2];
> + uint32_t mask[2];
> + uint32_t wumask;
> + uint32_t ints[2];
> + uint32_t debounce;
> + uint8_t delay;
> +};
> +
> +struct omap_gpif_s {
> + SysBusDevice busdev;
> + int mpu_model;
> + union {
> + struct omap_gpio_s omap1;
> + struct {
> + int modulecount;
> + struct omap2_gpio_s *modules;
> + qemu_irq *handler;
> + int autoidle;
> + int gpo;
> + } omap2;
> + } regs;
> +};
> +
> +/* General-Purpose I/O of OMAP1 */
> static void omap_gpio_set(void *opaque, int line, int level)
> {
> - struct omap_gpio_s *s = (struct omap_gpio_s *) opaque;
> + struct omap_gpio_s *s = &((struct omap_gpif_s *)opaque)->regs.omap1;
> uint16_t prev = s->inputs;
>
> if (level)
> @@ -160,7 +195,7 @@ static CPUWriteMemoryFunc * const omap_gpio_writefn[] = {
> omap_badwidth_write16,
> };
>
> -void omap_gpio_reset(struct omap_gpio_s *s)
> +static void omap_gpio_reset(struct omap_gpio_s *s)
> {
> s->inputs = 0;
> s->outputs = ~0;
> @@ -171,58 +206,9 @@ void omap_gpio_reset(struct omap_gpio_s *s)
> s->pins = ~0;
> }
>
> -struct omap_gpio_s *omap_gpio_init(target_phys_addr_t base,
> - qemu_irq irq, omap_clk clk)
> -{
> - int iomemtype;
> - struct omap_gpio_s *s = (struct omap_gpio_s *)
> - qemu_mallocz(sizeof(struct omap_gpio_s));
> -
> - s->irq = irq;
> - s->in = qemu_allocate_irqs(omap_gpio_set, s, 16);
> - omap_gpio_reset(s);
> -
> - iomemtype = cpu_register_io_memory(omap_gpio_readfn,
> - omap_gpio_writefn, s, DEVICE_NATIVE_ENDIAN);
> - cpu_register_physical_memory(base, 0x1000, iomemtype);
> -
> - return s;
> -}
> -
> -qemu_irq *omap_gpio_in_get(struct omap_gpio_s *s)
> -{
> - return s->in;
> -}
> -
> -void omap_gpio_out_set(struct omap_gpio_s *s, int line, qemu_irq handler)
> -{
> - if (line >= 16 || line < 0)
> - hw_error("%s: No GPIO line %i\n", __FUNCTION__, line);
> - s->handler[line] = handler;
> -}
> -
> -/* General-Purpose Interface of OMAP2 */
> -struct omap2_gpio_s {
> - qemu_irq irq[2];
> - qemu_irq wkup;
> - qemu_irq *in;
> - qemu_irq handler[32];
> -
> - uint8_t config[2];
> - uint32_t inputs;
> - uint32_t outputs;
> - uint32_t dir;
> - uint32_t level[2];
> - uint32_t edge[2];
> - uint32_t mask[2];
> - uint32_t wumask;
> - uint32_t ints[2];
> - uint32_t debounce;
> - uint8_t delay;
> -};
> -
> +/* General-Purpose Interface of OMAP2/3 */
> static inline void omap2_gpio_module_int_update(struct omap2_gpio_s *s,
> - int line)
> + int line)
> {
> qemu_set_irq(s->irq[line], s->ints[line] & s->mask[line]);
> }
> @@ -271,8 +257,9 @@ static inline void omap2_gpio_module_int(struct
> omap2_gpio_s *s, int line)
>
> static void omap2_gpio_module_set(void *opaque, int line, int level)
> {
> - struct omap2_gpio_s *s = (struct omap2_gpio_s *) opaque;
> -
> + struct omap_gpif_s *p = opaque;
> + struct omap2_gpio_s *s = &p->regs.omap2.modules[line >> 5];
> + line &= 31;
Patch looks good overall, but for consistency we should rename
functions which start with omap2_gpio_module_ to omap2_gpio_ if the
state pointer passed is no longer the module pointer but instead the
whole thing pointer. But maybe it would make more sense for each
module to be a device? It looks like vmsaving/vmloading the union
structure may be problematic and we can also save some cycles.
Similarly omap_l4_base would be better named omap_l4_region_base or
similar. I'd also prefer that omap2_gpio_init remains a function that
does all the qdev magic in omap_gpio.c and with the current signature
(so for example clocks usage can be kept track of). Would it make
more sense to pass the ta structures instead of base adresses to the
qdev? Have you considered how difficult l4 would be to convert to a
QBus?
Cheers
- Re: [Qemu-devel] [PATCH 3/3] hw/omap_gpio.c: Convert to qdev,
andrzej zaborowski <=