qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V2 2/3] exynos4210: add exynos4210 GPIO implemen


From: Igor Mitsyanko
Subject: Re: [Qemu-devel] [PATCH V2 2/3] exynos4210: add exynos4210 GPIO implementation
Date: Wed, 21 Mar 2012 17:01:08 +0400
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.27) Gecko/20120216 Thunderbird/3.1.19

On 03/20/2012 05:27 PM, Peter Maydell wrote:
On 15 March 2012 07:35, Igor Mitsyanko<address@hidden>  wrote:
Now that we have GPIO emulation for exynos4210 SoC we can use it to
properly hook up IRQ line to lan9215 controller on SMDK board.

+#elif EXYNOS4210_GPIO_DEBUG == 1
+#define DPRINT_L1(fmt, args...)       \
+    do {fprintf(stderr, "QEMU GPIO: "fmt, ## args); } while (0)

Space after the closing quote in these macros (this patch and
others), please.

+
+    DPRINT_L1("Input pin GPIO%s_PIN%u %s by external device\n",
+            g->ports[group_num].name, pin, (level ? "set" : "cleared"));
+    /* Set new value on corresponding gpio pin */
+    (level) ? (g->ports[group_num].dat |= (1<<  pin)) :
+            (g->ports[group_num].dat&= ~(1<<  pin));

Don't use ?: like this please, just write it out as an if statement.


OK

+static void exynos4_gpio_write(void *opaque, target_phys_addr_t off,
+                               uint64_t value, unsigned size)
+{

I find these read and write functions quite hard to read. I'm
wondering if it would work better to split them up into smaller
memory regions which get registered at the right addresses, rather
than effectively doing the decode into different subsections based
on address and g->part here?


Initially I implemented these functions splitted like you suggest, but that left me with three basically identical long switch statements, so I decided to combine them into one to achieve better code reuse. If you think code reuse is not an issue here, I'll gladly return it back to splitted implementation.

+    Exynos4GpioState *g = (Exynos4GpioState *)opaque;
+    unsigned int port_end, extint_con_start, extint_con_end;
+    unsigned int extint_flt_start, extint_flt_end;
+    unsigned int extint_mask_start, extint_mask_end;
+    unsigned int extint_pend_start, extint_pend_end;
+    unsigned int etcp_start_addr, etcp_start_idx, extint_pri_end;
+    unsigned idx;
+
+    DPRINT_L2("GPIO%u write off 0x%x = %u(0x%x)\n", g->part,
+            (uint32_t)off, (uint32_t)value, (uint32_t)value);
+
+    switch (g->part) {
+    case GPIO_PART2X:
+        port_end = GPIO2_XPORT_END;
+        extint_con_start = GPIO2_WKPINT_CON_START;
+        extint_con_end = GPIO2_WKPINT_CON_END;
+        extint_flt_start = GPIO2_WKPINT_FLT_START;
+        extint_flt_end = GPIO2_WKPINT_FLT_END;
+        extint_mask_start = GPIO2_WKPINT_MASK_START;
+        extint_mask_end = GPIO2_WKPINT_MASK_END;
+        extint_pend_start = GPIO2_WKPINT_PEND_START;
+        extint_pend_end = GPIO2_WKPINT_PEND_END;
+        etcp_start_addr = etcp_start_idx = extint_pri_end = 0;
+        break;
+    case GPIO_PART1: default:
+        port_end = GPIO_NORM_PORT_END;
+        extint_con_start = GPIO_EXTINT_CON_START;
+        extint_con_end = GPIO1_EXTINT_CON_END;
+        extint_flt_start = GPIO_EXTINT_FLT_START;
+        extint_flt_end = GPIO1_EXTINT_FLT_END;
+        extint_mask_start = GPIO_EXTINT_MASK_START;
+        extint_mask_end = GPIO1_EXTINT_MASK_END;
+        extint_pend_start = GPIO_EXTINT_PEND_START;
+        extint_pend_end = GPIO1_EXTINT_PEND_END;
+        etcp_start_addr = GPIO1_ETCPORT_START;
+        etcp_start_idx = GPIO1_NORM_PORT_NUM;
+        extint_pri_end = GPIO1_EXTINT_FIXPRI_END;
+        break;
+    case GPIO_PART2:
+        port_end = GPIO_NORM_PORT_END;
+        extint_con_start = GPIO_EXTINT_CON_START;
+        extint_con_end = GPIO2_EXTINT_CON_END;
+        extint_flt_start = GPIO_EXTINT_FLT_START;
+        extint_flt_end = GPIO2_EXTINT_FLT_END;
+        extint_mask_start = GPIO_EXTINT_MASK_START;
+        extint_mask_end = GPIO2_EXTINT_MASK_END;
+        extint_pend_start = GPIO_EXTINT_PEND_START;
+        extint_pend_end = GPIO2_EXTINT_PEND_END;
+        etcp_start_addr = GPIO2_ETCPORT_START;
+        etcp_start_idx = GPIO2_NORM_PORT_NUM;
+        extint_pri_end = GPIO2_EXTINT_FIXPRI_END;
+        break;
+    case GPIO_PART3:
+        if (off<  GPIO3_NORM_PORT_END) {
+            idx = DIV_BY_PORTGR_SIZE(off);
+            exynos4_gpio_portgr_write(g, idx, MOD_PORTGR_SIZE(off), value);
+        } else {
+            DPRINT_ERROR("GPIO3 bad write off 0x%x = %u(0x%x)\n",
+                    (uint32_t)off, (uint32_t)value, (uint32_t)value);
+        }
+        return;
+    }
+
+    if (off<  port_end) {
+        idx = DIV_BY_PORTGR_SIZE(off);
+        exynos4_gpio_portgr_write(g, idx, MOD_PORTGR_SIZE(off), value);
+    } else if (off>= extint_mask_start&&  off<  extint_mask_end) {
+        idx = (off - extint_mask_start)>>  2;
+        DPRINT_L1("GPIO%u EXTINT%u_MASK register write = %u(0x%x)\n", g->part,
+            g->port_ints[idx].int_line_num, (uint32_t)value, (uint32_t)value);
+        g->port_ints[idx].mask = value;
+    } else if (off>= extint_pend_start&&  off<  extint_pend_end) {
+        idx = (off - extint_pend_start)>>  2;
+        DPRINT_L1("GPIO%u EXTINT%u_PEND register write = %u(0x%x)\n", g->part,
+            g->port_ints[idx].int_line_num, (uint32_t)value, (uint32_t)value);
+        if (g->part == GPIO_PART2X) {
+            unsigned i, irq_n;
+            Exynos4Gpio2XState *g2 = EXYNOS4210_GPIO2X(g);
+            for (i = 0; i<  GPIO_MAX_PIN_IN_PORT; i++) {
+                if ((g->port_ints[idx].pend&  (1<<  i))&&  (value&  (1<<  i))) 
{
+                    g->port_ints[idx].pend&= ~(1<<  i);
+                    irq_n = idx * GPIO_MAX_PIN_IN_PORT + i;
+                    if (irq_n>= GPIO2_X_PORT_IRQ_NUM) {
+                        irq_n = GPIO2_X_PORT_IRQ_NUM - 1;
+                    }
+                    qemu_irq_lower(g2->ext_irq[irq_n]);
+                }
+            }
+        } else {
+            g->port_ints[idx].pend&= ~value;
+            exynos4_gpioirq_update(g);
+        }
+    } else if (off>= extint_con_start&&  off<  extint_con_end) {
+        idx = (off - extint_con_start)>>  2;
+        DPRINT_L1("GPIO%u EXTINT%u_CON register write = %u(0x%x)\n", g->part,
+            g->port_ints[idx].int_line_num, (uint32_t)value, (uint32_t)value);
+        g->port_ints[idx].con = value;
+    } else if (off>= extint_flt_start&&  off<  extint_flt_end) {
+        unsigned i = ((off - extint_flt_start)>>  2)&  1;
+        idx = (off - extint_flt_start)>>  3;
+        DPRINT_L1("GPIO%u EXTINT%u_FLTCON%u reg write = %u(0x%x)\n", g->part,
+          g->port_ints[idx].int_line_num, i, (uint32_t)value, (uint32_t)value);
+        g->port_ints[idx].fltcon[i] = value;
+    } else if (g->part == GPIO_PART2X) {
+        DPRINT_ERROR("GPIO2 group X bad write off 0x%x = %u(0x%x)\n",
+                (uint32_t)off, (uint32_t)value, (uint32_t)value);
+        return;
+    } else if (off == GPIO_EXTINT_SERVICE) {
+        DPRINT_L1("GPIO%u EXT_INT_SERVICE register write = %u(0x%x)\n",
+                g->part, (uint32_t)value, (uint32_t)value);
+        EXYNOS4210_GPIO_1_2(g)->extint_serv = value;
+    } else if (off == GPIO_EXTINT_SERVICE_PEND) {
+        DPRINT_L1("GPIO%u EXT_INT_SERVICE_PEND register write = %u(0x%x)\n",
+                g->part, (uint32_t)value, (uint32_t)value);
+        EXYNOS4210_GPIO_1_2(g)->extint_serv_pend = value;
+    } else if (off == GPIO_EXTINT_GRPFIXPRI) {
+        DPRINT_L1("GPIO%u EXT_INT_GRPFIXPRI register write = %u(0x%x)\n",
+                g->part, (uint32_t)value, (uint32_t)value);
+        EXYNOS4210_GPIO_1_2(g)->extint_grpfixpri = value;
+    } else if (off>= GPIO_EXTINT_FIXPRI_START&&  off<  extint_pri_end) {
+        idx = (off - GPIO_EXTINT_FIXPRI_START)>>  2;
+        DPRINT_L1("GPIO%u EXTINT%u_FIXPRI register write = %u(0x%x)\n", 
g->part,
+            g->port_ints[idx].int_line_num, (uint32_t)value, (uint32_t)value);
+        g->port_ints[idx].fixpri = value;
+    } else if (off>= etcp_start_addr&&  off<  GPIO_ETCPORT_END) {
+        idx = etcp_start_idx + DIV_BY_PORTGR_SIZE(off - etcp_start_addr);
+        exynos4_etc_portgroup_write(&g->ports[idx],
+            MOD_PORTGR_SIZE(off - etcp_start_addr), value);
+    } else {
+        DPRINT_ERROR("GPIO%u bad write off 0x%x = %u(0x%x)\n", g->part,
+            (uint32_t)off, (uint32_t)value, (uint32_t)value);
+    }
+}
+static void exynos4_gpio1_init(Object *obj)
+{
+    Exynos4GpioState *g = EXYNOS4210_GPIO(obj);
+
+    g->part = GPIO_PART1;
+    g->ports = gpio1_ports;
+    g->port_ints = gpio1_ports_interrupts;
+    g->num_of_ports = sizeof(gpio1_ports) / sizeof(Exynos4PortGroup);
+    g->num_of_portints =
+            sizeof(gpio1_ports_interrupts) / sizeof(Exynos4PortIntState);

You can use the ARRAY_SIZE macro here.

-- PMM




--
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: address@hidden



reply via email to

[Prev in Thread] Current Thread [Next in Thread]