Re: [PATCHv2 RESEND] irqchip: Add support for TI-NSPIRE irqchip

2013-12-04 Thread Thomas Gleixner
On Mon, 25 Nov 2013, dt.ta...@gmail.com wrote:
> diff --git a/drivers/irqchip/irq-zevio.c b/drivers/irqchip/irq-zevio.c
> +static void zevio_irq_ack(struct irq_data *irqd)
> +{
> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(irqd);
> + struct irq_chip_regs *regs =
> + &container_of(irqd->chip, struct irq_chip_type, chip)->regs;
> +
> + irq_gc_lock(gc);

Why do you need to lock here? You are reading the ack register and not
modifying it. You are neither storing the value in the gc itself. So
what are you trying to protect?

> + readl(gc->reg_base + regs->ack);
> + irq_gc_unlock(gc);
> +}
> +
> +static void init_base(void __iomem *base)

Shouldnt that be marked __init ?

> +{
> + /* Disable all interrupts */
> + writel(~0, base + IO_DISABLE);

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv2 RESEND] irqchip: Add support for TI-NSPIRE irqchip

2013-12-04 Thread Daniel Tang
Hi,

I've noticed this patch hasn't gotten a reply for a while now. Just wondering 
what's the status of this patch and whether there is anything else I should fix 
before this can get accepted.

Cheers,
Daniel Tang

On 25/11/2013, at 3:02 PM, dt.ta...@gmail.com wrote:

> From: Daniel Tang 
> 
> This patch adds support for the interrupt controllers found in some
> TI-Nspire models.
> 
> FIQ support was taken out to simplify the driver
> code and may be added in later. Since Linux on this platform doesn't
> really use FIQs, this wasn't really that important in the first
> place.
> 
> Changes from v1 to v2:
> * Converted to use generic IRQ chips.
> * Removed FIQ for now to simplify driver code.
> * Based against tip/irq/core and uses IRQ domain support for generic
> chips.
> 
> Signed-off-by: Daniel Tang 
> Acked-by: Grant Likely 
> ---
> .../interrupt-controller/lsi,zevio-intc.txt|  18 +++
> drivers/irqchip/Makefile   |   1 +
> drivers/irqchip/irq-zevio.c| 129 +
> 3 files changed, 148 insertions(+)
> create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/lsi,zevio-intc.txt
> create mode 100644 drivers/irqchip/irq-zevio.c
> 
> diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/lsi,zevio-intc.txt 
> b/Documentation/devicetree/bindings/interrupt-controller/lsi,zevio-intc.txt
> new file mode 100644
> index 000..aee38e7
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/lsi,zevio-intc.txt
> @@ -0,0 +1,18 @@
> +TI-NSPIRE interrupt controller
> +
> +Required properties:
> +- compatible: Compatible property value should be "lsi,zevio-intc".
> +
> +- reg: Physical base address of the controller and length of memory mapped
> + region.
> +
> +- interrupt-controller : Identifies the node as an interrupt controller
> +
> +Example:
> +
> +interrupt-controller {
> + compatible = "lsi,zevio-intc";
> + interrupt-controller;
> + reg = <0xDC00 0x1000>;
> + #interrupt-cells = <1>;
> +};
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index cda4cb5..f313d14 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -15,4 +15,5 @@ obj-$(CONFIG_SIRF_IRQ)  += irq-sirfsoc.o
> obj-$(CONFIG_RENESAS_INTC_IRQPIN) += irq-renesas-intc-irqpin.o
> obj-$(CONFIG_RENESAS_IRQC)+= irq-renesas-irqc.o
> obj-$(CONFIG_VERSATILE_FPGA_IRQ)  += irq-versatile-fpga.o
> +obj-$(CONFIG_ARCH_NSPIRE)+= irq-zevio.o
> obj-$(CONFIG_ARCH_VT8500) += irq-vt8500.o
> diff --git a/drivers/irqchip/irq-zevio.c b/drivers/irqchip/irq-zevio.c
> new file mode 100644
> index 000..92e6c7b
> --- /dev/null
> +++ b/drivers/irqchip/irq-zevio.c
> @@ -0,0 +1,129 @@
> +/*
> + *  linux/drivers/irqchip/irq-zevio.c
> + *
> + *  Copyright (C) 2013 Daniel Tang 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "irqchip.h"
> +
> +#define IO_STATUS0x000
> +#define IO_RAW_STATUS0x004
> +#define IO_ENABLE0x008
> +#define IO_DISABLE   0x00C
> +#define IO_CURRENT   0x020
> +#define IO_RESET 0x028
> +#define IO_MAX_PRIOTY0x02C
> +
> +#define IO_IRQ_BASE  0x000
> +#define IO_FIQ_BASE  0x100
> +
> +#define IO_INVERT_SEL0x200
> +#define IO_STICKY_SEL0x204
> +#define IO_PRIORITY_SEL  0x300
> +
> +#define MAX_INTRS32
> +#define FIQ_STARTMAX_INTRS
> +
> +static struct irq_domain *zevio_irq_domain;
> +static void __iomem *zevio_irq_io;
> +
> +static void zevio_irq_ack(struct irq_data *irqd)
> +{
> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(irqd);
> + struct irq_chip_regs *regs =
> + &container_of(irqd->chip, struct irq_chip_type, chip)->regs;
> +
> + irq_gc_lock(gc);
> + readl(gc->reg_base + regs->ack);
> + irq_gc_unlock(gc);
> +}
> +
> +static void init_base(void __iomem *base)
> +{
> + /* Disable all interrupts */
> + writel(~0, base + IO_DISABLE);
> +
> + /* Accept interrupts of all priorities */
> + writel(0xF, base + IO_MAX_PRIOTY);
> +
> + /* Reset existing interrupts */
> + readl(base + IO_RESET);
> +}
> +
> +asmlinkage void __exception_irq_entry zevio_handle_irq(struct pt_regs *regs)
> +{
> + int irqnr;
> +
> + while (readl(zevio_irq_io + IO_STATUS)) {
> + irqnr = readl(zevio_irq_io + IO_CURRENT);
> + irqnr = irq_find_mapping(zevio_irq_domain, irqnr);
> + handle_IRQ(irqnr, regs);
> + };
> +}
> +
> +static int __init zevio_of_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + unsigned int clr = IRQ_NOREQUEST | IRQ