Re: [PATCH v4 15/18] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq

[Julien Grall <julien.grall@linaro.org>]

On 04/22/2014 01:58 PM, Julien Grall wrote:
> diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
> index 370539c..74ee578 100644
> --- a/xen/drivers/char/exynos4210-uart.c
> +++ b/xen/drivers/char/exynos4210-uart.c
> @@ -30,7 +30,7 @@

>  static const struct vuart_info *exynos4210_vuart_info(struct serial_port *port)
> @@ -323,12 +323,13 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
>          return res;
>      }
>  
> -    res = dt_device_get_irq(dev, 0, &uart->irq);
> -    if ( res )
> +    res = platform_get_irq(dev, 0);
> +    if ( uart->irq < 0 )

Argh, I test the wrong variable here. This should be

if ( res < 0 )

I've copied the slightly modify patch below.

commit 7a98180798deed5e453128bc3556cadef025d894
Author: Julien Grall <julien.grall@linaro.org>
Date:   Thu Mar 27 17:54:27 2014 +0000

    xen/arm: IRQ: Replace {request,setup}_dt_irq by {request,setup}_irq
    
    Now that irq_desc stores the type of the IRQ (e.g level/edge,...), we don't
    need to use specific IRQ function for ARM.
    
    Also replace every call to dt_device_get_irq by platform_get_irq which is
    a wrapper to this function and setup the IRQ type correctly.
    
    Signed-off-by: Julien Grall <julien.grall@linaro.org>
    Acked-by: Ian Campbell <ian.campbell@citrix.com>
    Cc: Keir Fraser <keir@xen.org>
    
    ---
        Changes in v5:
            - exynos4210 was testing the wrong IRQ variable
    
        Changes in v4:
            - platform_get_irq now returns -1 in case of error.
    
        Changes in v3:
            - Fix typoes in commit message
    
        Changes in v2:
            - Patch added

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 4a6ed35..cb40b0a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -47,7 +47,7 @@ static struct {
     paddr_t hbase;       /* Address of virtual interface registers */
     paddr_t vbase;       /* Address of virtual cpu interface registers */
     unsigned int lines;  /* Number of interrupts (SPIs + PPIs + SGIs) */
-    struct dt_irq maintenance; /* IRQ maintenance */
+    unsigned int maintenance_irq; /* IRQ maintenance */
     unsigned int cpus;
     spinlock_t lock;
 } gic;
@@ -431,9 +431,10 @@ void __init gic_init(void)
     if ( res || !gic.vbase || (gic.vbase & ~PAGE_MASK) )
         panic("GIC: Cannot find a valid address for the virtual CPU");
 
-    res = dt_device_get_irq(node, 0, &gic.maintenance);
-    if ( res )
+    res = platform_get_irq(node, 0);
+    if ( res < 0 )
         panic("GIC: Cannot find the maintenance IRQ");
+    gic.maintenance_irq = res;
 
     /* Set the GIC as the primary interrupt controller */
     dt_interrupt_controller = node;
@@ -447,7 +448,7 @@ void __init gic_init(void)
               "        gic_vcpu_addr=%"PRIpaddr"\n"
               "        gic_maintenance_irq=%u\n",
               gic.dbase, gic.cbase, gic.hbase, gic.vbase,
-              gic.maintenance.irq);
+              gic.maintenance_irq);
 
     if ( (gic.dbase & ~PAGE_MASK) || (gic.cbase & ~PAGE_MASK) ||
          (gic.hbase & ~PAGE_MASK) || (gic.vbase & ~PAGE_MASK) )
@@ -884,8 +885,8 @@ void gic_dump_info(struct vcpu *v)
 
 void __cpuinit init_maintenance_interrupt(void)
 {
-    request_dt_irq(&gic.maintenance, maintenance_interrupt,
-                   "irq-maintenance", NULL);
+    request_irq(gic.maintenance_irq, maintenance_interrupt,
+                "irq-maintenance", NULL);
 }
 
 /*
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index f1d6398..a09ca25 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -117,9 +117,9 @@ static inline struct domain *irq_get_domain(struct irq_desc *desc)
     return desc->action->dev_id;
 }
 
-int request_dt_irq(const struct dt_irq *irq,
-                   void (*handler)(int, void *, struct cpu_user_regs *),
-                   const char *devname, void *dev_id)
+int request_irq(unsigned int irq,
+                void (*handler)(int, void *, struct cpu_user_regs *),
+                const char *devname, void *dev_id)
 {
     struct irqaction *action;
     int retval;
@@ -130,13 +130,13 @@ int request_dt_irq(const struct dt_irq *irq,
      * which interrupt is which (messes up the interrupt freeing
      * logic etc).
      */
-    if (irq->irq >= nr_irqs)
+    if ( irq >= nr_irqs )
         return -EINVAL;
-    if (!handler)
+    if ( !handler )
         return -EINVAL;
 
     action = xmalloc(struct irqaction);
-    if (!action)
+    if ( !action )
         return -ENOMEM;
 
     action->handler = handler;
@@ -144,8 +144,8 @@ int request_dt_irq(const struct dt_irq *irq,
     action->dev_id = dev_id;
     action->free_on_release = 1;
 
-    retval = setup_dt_irq(irq, action);
-    if (retval)
+    retval = setup_irq(irq, action);
+    if ( retval )
         xfree(action);
 
     return retval;
@@ -252,14 +252,14 @@ static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
     return 0;
 }
 
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
+int setup_irq(unsigned int irq, struct irqaction *new)
 {
     int rc;
     unsigned long flags;
     struct irq_desc *desc;
     bool_t disabled;
 
-    desc = irq_to_desc(irq->irq);
+    desc = irq_to_desc(irq);
 
     spin_lock_irqsave(&desc->lock, flags);
 
@@ -269,7 +269,7 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
 
         spin_unlock_irqrestore(&desc->lock, flags);
         printk(XENLOG_ERR "ERROR: IRQ %u is already in use by the domain %u\n",
-               irq->irq, d->domain_id);
+               irq, d->domain_id);
         return -EBUSY;
     }
 
@@ -289,7 +289,6 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
          * TODO: Handle case where SPI is setup on different CPU than
          * the targeted CPU and the priority.
          */
-        desc->arch.type = irq->type;
         gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
                              GIC_PRI_IRQ);
         desc->handler->startup(desc);
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index ce96337..0bff747 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -48,13 +48,13 @@ uint64_t __read_mostly boot_count;
  * register-mapped time source in the SoC. */
 unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
 
-static struct dt_irq timer_irq[MAX_TIMER_PPI];
+static unsigned int timer_irq[MAX_TIMER_PPI];
 
 unsigned int timer_get_irq(enum timer_ppi ppi)
 {
     ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
 
-    return timer_irq[ppi].irq;
+    return timer_irq[ppi];
 }
 
 /*static inline*/ s_time_t ticks_to_ns(uint64_t ticks)
@@ -120,15 +120,17 @@ int __init init_xen_time(void)
     /* Retrieve all IRQs for the timer */
     for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
     {
-        res = dt_device_get_irq(dev, i, &timer_irq[i]);
-        if ( res )
+        res = platform_get_irq(dev, i);
+
+        if ( res < 0 )
             panic("Timer: Unable to retrieve IRQ %u from the device tree", i);
+        timer_irq[i] = res;
     }
 
     printk("Generic Timer IRQ: phys=%u hyp=%u virt=%u\n",
-           timer_irq[TIMER_PHYS_NONSECURE_PPI].irq,
-           timer_irq[TIMER_HYP_PPI].irq,
-           timer_irq[TIMER_VIRT_PPI].irq);
+           timer_irq[TIMER_PHYS_NONSECURE_PPI],
+           timer_irq[TIMER_HYP_PPI],
+           timer_irq[TIMER_VIRT_PPI]);
 
     res = platform_init_time();
     if ( res )
@@ -192,7 +194,7 @@ int reprogram_timer(s_time_t timeout)
 /* Handle the firing timer */
 static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 {
-    if ( irq == (timer_irq[TIMER_HYP_PPI].irq) &&
+    if ( irq == (timer_irq[TIMER_HYP_PPI]) &&
          READ_SYSREG32(CNTHP_CTL_EL2) & CNTx_CTL_PENDING )
     {
         /* Signal the generic timer code to do its work */
@@ -201,7 +203,7 @@ static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
         WRITE_SYSREG32(0, CNTHP_CTL_EL2);
     }
 
-    if ( irq == (timer_irq[TIMER_PHYS_NONSECURE_PPI].irq) &&
+    if ( irq == (timer_irq[TIMER_PHYS_NONSECURE_PPI]) &&
          READ_SYSREG32(CNTP_CTL_EL0) & CNTx_CTL_PENDING )
     {
         /* Signal the generic timer code to do its work */
@@ -235,12 +237,12 @@ void __cpuinit init_timer_interrupt(void)
     WRITE_SYSREG32(0, CNTHP_CTL_EL2);   /* Hypervisor's timer disabled */
     isb();
 
-    request_dt_irq(&timer_irq[TIMER_HYP_PPI], timer_interrupt,
-                   "hyptimer", NULL);
-    request_dt_irq(&timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
+    request_irq(timer_irq[TIMER_HYP_PPI], timer_interrupt,
+                "hyptimer", NULL);
+    request_irq(timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
                    "virtimer", NULL);
-    request_dt_irq(&timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
-                   "phytimer", NULL);
+    request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
+                "phytimer", NULL);
 }
 
 /* Wait a set number of microseconds */
diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index 370539c..404ce05 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -30,7 +30,7 @@
 
 static struct exynos4210_uart {
     unsigned int baud, clock_hz, data_bits, parity, stop_bits;
-    struct dt_irq irq;
+    unsigned int irq;
     void *regs;
     struct irqaction irqaction;
     struct vuart_info vuart;
@@ -197,9 +197,9 @@ static void __init exynos4210_uart_init_postirq(struct serial_port *port)
     uart->irqaction.name    = "exynos4210_uart";
     uart->irqaction.dev_id  = port;
 
-    if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
+    if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
         dprintk(XENLOG_ERR, "Failed to allocated exynos4210_uart IRQ %d\n",
-                uart->irq.irq);
+                uart->irq);
 
     /* Unmask interrupts */
     exynos4210_write(uart, UINTM, ~UINTM_ALLI);
@@ -272,7 +272,7 @@ static int __init exynos4210_uart_irq(struct serial_port *port)
 {
     struct exynos4210_uart *uart = port->uart;
 
-    return uart->irq.irq;
+    return uart->irq;
 }
 
 static const struct vuart_info *exynos4210_vuart_info(struct serial_port *port)
@@ -323,12 +323,13 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( res < 0 )
     {
         printk("exynos4210: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 21f086a..6691806 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -76,9 +76,6 @@ static struct ns16550 {
     u8 bar_idx;
     bool_t enable_ro; /* Make MMIO devices read only to Dom0 */
 #endif
-#ifdef HAS_DEVICE_TREE
-    struct dt_irq dt_irq;
-#endif
 } ns16550_com[2] = { { 0 } };
 
 struct ns16550_config_mmio {
@@ -612,13 +609,8 @@ static void __init ns16550_init_postirq(struct serial_port *port)
         uart->irqaction.handler = ns16550_interrupt;
         uart->irqaction.name    = "ns16550";
         uart->irqaction.dev_id  = port;
-#ifdef HAS_DEVICE_TREE
-        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
-            printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
-#else
         if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
             printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
-#endif
     }
 
     ns16550_setup_postirq(uart);
@@ -1172,12 +1164,10 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
     if ( uart->reg_width != 1 && uart->reg_width != 4 )
         return -EINVAL;
 
-    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
-    if ( res )
-        return res;
-
-    /* The common bit of the driver mostly deals with irq not dt_irq. */
-    uart->irq = uart->dt_irq.irq;
+    res = platform_get_irq(dev, 0);
+    if ( ! res )
+        return -EINVAL;
+    uart->irq = res;
 
     uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
 
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index b8da509..e598785 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -30,7 +30,7 @@
 
 static struct omap_uart {
     u32 baud, clock_hz, data_bits, parity, stop_bits, fifo_size;
-    struct dt_irq irq;
+    unsigned int irq;
     char __iomem *regs;
     struct irqaction irqaction;
     struct vuart_info vuart;
@@ -205,10 +205,10 @@ static void __init omap_uart_init_postirq(struct serial_port *port)
     uart->irqaction.name = "omap_uart";
     uart->irqaction.dev_id = port;
 
-    if ( setup_dt_irq(&uart->irq, &uart->irqaction) != 0 )
+    if ( setup_irq(uart->irq, &uart->irqaction) != 0 )
     {
         dprintk(XENLOG_ERR, "Failed to allocated omap_uart IRQ %d\n",
-                uart->irq.irq);
+                uart->irq);
         return;
     }
 
@@ -259,7 +259,7 @@ static int __init omap_uart_irq(struct serial_port *port)
 {
     struct omap_uart *uart = port->uart;
 
-    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
+    return ((uart->irq > 0) ? uart->irq : -1);
 }
 
 static const struct vuart_info *omap_vuart_info(struct serial_port *port)
@@ -317,12 +317,13 @@ static int __init omap_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( res < 0 )
     {
         printk("omap-uart: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 459e686..89bda94 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -32,7 +32,7 @@
 
 static struct pl011 {
     unsigned int baud, clock_hz, data_bits, parity, stop_bits;
-    struct dt_irq irq;
+    unsigned int irq;
     void __iomem *regs;
     /* UART with IRQ line: interrupt-driven I/O. */
     struct irqaction irqaction;
@@ -132,13 +132,13 @@ static void __init pl011_init_postirq(struct serial_port *port)
     struct pl011 *uart = port->uart;
     int rc;
 
-    if ( uart->irq.irq > 0 )
+    if ( uart->irq > 0 )
     {
         uart->irqaction.handler = pl011_interrupt;
         uart->irqaction.name    = "pl011";
         uart->irqaction.dev_id  = port;
-        if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
-            printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq.irq);
+        if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+            printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
     }
 
     /* Clear pending error interrupts */
@@ -186,7 +186,8 @@ static int pl011_getc(struct serial_port *port, char *pc)
 static int __init pl011_irq(struct serial_port *port)
 {
     struct pl011 *uart = port->uart;
-    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
+
+    return ((uart->irq > 0) ? uart->irq : -1);
 }
 
 static const struct vuart_info *pl011_vuart(struct serial_port *port)
@@ -239,12 +240,13 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( res < 0 )
     {
         printk("pl011: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 93d2128..f9b9a9f 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -40,11 +40,6 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq);
 void init_IRQ(void);
 void init_secondary_IRQ(void);
 
-int request_dt_irq(const struct dt_irq *irq,
-                   void (*handler)(int, void *, struct cpu_user_regs *),
-                   const char *devname, void *dev_id);
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
-
 int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
                           const char *devname);
 int platform_get_irq(const struct dt_device_node *device,

-- 
Julien Grall