RE: [PATCH V2 2/7] DRIVERS: IRQCHIP: CROSSBAR: Add support for Crossbar IP

2013-10-31 Thread R, Sricharan
Hi Thomas,
  Again sorry for top post. 

  I agree and will fix both of the comments below.

  Thanks for the reviews.

Regards,
 Sricharan

From: Thomas Gleixner [t...@linutronix.de]
Sent: Wednesday, October 30, 2013 9:00 PM
To: R, Sricharan
Cc: linux-ker...@vger.kernel.org; devicet...@vger.kernel.org; 
linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
linux-omap@vger.kernel.org; linus.wall...@linaro.org; li...@arm.linux.org.uk; 
t...@atomide.com; Nayak, Rajendra; marc.zyng...@arm.com; 
grant.lik...@linaro.org; mark.rutl...@arm.com; robherri...@gmail.com; 
Shilimkar, Santosh; Rob Herring
Subject: Re: [PATCH V2 2/7] DRIVERS: IRQCHIP: CROSSBAR: Add support for 
Crossbar IP

On Wed, 30 Oct 2013, Sricharan R wrote:
 +static inline const u32 allocate_free_irq(int cb_no)

I understand the static inline part, but const u32 is more than
fishy. What's wrong with static inline int ?

 +{
 + int i;
 +
 + for (i = 0; i  cb-int_max; i++) {
 + if (cb-irq_map[i] == IRQ_FREE) {
 + cb-irq_map[i] = cb_no;
 + return i;
 + }
 + }
 +
 + return -ENODEV;
 +}

 +static int crossbar_domain_xlate(struct irq_domain *d,
 +  struct device_node *controller,
 +  const u32 *intspec, unsigned int intsize,
 +  unsigned long *out_hwirq,
 +  unsigned int *out_type)
 +{
 + unsigned long ret = 0;

Why do you need to initialize ret when you assign a value to it in the
next line?

Thanks,

tglx--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V2 2/7] DRIVERS: IRQCHIP: CROSSBAR: Add support for Crossbar IP

2013-10-30 Thread Sricharan R
Some socs have a large number of interrupts requests to service
the needs of its many peripherals and subsystems. All of the
interrupt lines from the subsystems are not needed at the same
time, so they have to be muxed to the irq-controller appropriately.
In such places a interrupt controllers are preceded by an CROSSBAR
that provides flexibility in muxing the device requests to the controller
inputs.

This driver takes care a allocating a free irq and then configuring the
crossbar IP as a part of the mpu's irqchip callbacks. crossbar_init should
be called right before the irqchip_init, so that it is setup to handle the
irqchip callbacks.

Cc: Thomas Gleixner t...@linutronix.de
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Russell King li...@arm.linux.org.uk
Cc: Tony Lindgren t...@atomide.com
Cc: Rajendra Nayak rna...@ti.com
Cc: Marc Zyngier marc.zyng...@arm.com
Cc: Grant Likely grant.lik...@linaro.org
Cc: Rob Herring rob.herr...@calxeda.com
Signed-off-by: Sricharan R r.sricha...@ti.com
---
 [V2] Addressed Thomas Gleixner t...@linutronix.de comments
  and renamed the bindings as per Kumar Gala ga...@codeaurora.org
  comments.

 .../devicetree/bindings/arm/omap/crossbar.txt  |   27 +++
 drivers/irqchip/Kconfig|8 +
 drivers/irqchip/Makefile   |1 +
 drivers/irqchip/irq-crossbar.c |  206 
 include/linux/irqchip/irq-crossbar.h   |   11 ++
 5 files changed, 253 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/crossbar.txt
 create mode 100644 drivers/irqchip/irq-crossbar.c
 create mode 100644 include/linux/irqchip/irq-crossbar.h

diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt 
b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
new file mode 100644
index 000..fb88585
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
@@ -0,0 +1,27 @@
+Some socs have a large number of interrupts requests to service
+the needs of its many peripherals and subsystems. All of the
+interrupt lines from the subsystems are not needed at the same
+time, so they have to be muxed to the irq-controller appropriately.
+In such places a interrupt controllers are preceded by an CROSSBAR
+that provides flexibility in muxing the device requests to the controller
+inputs.
+
+Required properties:
+- compatible : Should be ti,irq-crossbar
+- reg: Base address and the size of the crossbar registers.
+- ti,max-irqs: Total number of irqs available at the interrupt controller.
+- ti,reg-size: Size of a individual register in bytes. Every individual
+   register is assumed to be of same size. Valid sizes are 1, 2, 4.
+- ti,irqs-reserved: List of the reserved irq lines that are not muxed using
+crossbar. These interrupt lines are reserved in the soc,
+so crossbar bar driver should not consider them as free
+lines.
+
+Examples:
+   crossbar_mpu: @4a02 {
+   compatible = ti,irq-crossbar;
+   reg = 0x4a002a48 0x130;
+   ti,max-irqs = 160;
+   ti,reg-size = 2;
+   ti,irqs-reserved = 0 1 2 3 5 6 131 132 139 140;
+   };
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 3792a1a..2efcde6 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -61,3 +61,11 @@ config VERSATILE_FPGA_IRQ_NR
int
default 4
depends on VERSATILE_FPGA_IRQ
+
+config IRQ_CROSSBAR
+   bool
+   help
+ Support for a CROSSBAR ip that preceeds the main interrupt controller.
+ The primary irqchip invokes the crossbar's callback which inturn 
allocates
+ a free irq and configures the IP. Thus the peripheral interrupts are
+ routed to one of the free irqchip interrupt lines.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index c60b901..2edead9 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_RENESAS_IRQC)+= irq-renesas-irqc.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)   += irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_VT8500)  += irq-vt8500.o
 obj-$(CONFIG_TB10X_IRQC)   += irq-tb10x.o
+obj-$(CONFIG_IRQ_CROSSBAR) += irq-crossbar.o
diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
new file mode 100644
index 000..2d355e4
--- /dev/null
+++ b/drivers/irqchip/irq-crossbar.c
@@ -0,0 +1,206 @@
+/*
+ *  drivers/irqchip/irq-crossbar.c
+ *
+ *  Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ *  Author: Sricharan R r.sricha...@ti.com
+ *
+ * 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.

Re: [PATCH V2 2/7] DRIVERS: IRQCHIP: CROSSBAR: Add support for Crossbar IP

2013-10-30 Thread Thomas Gleixner
On Wed, 30 Oct 2013, Sricharan R wrote:
 +static inline const u32 allocate_free_irq(int cb_no)

I understand the static inline part, but const u32 is more than
fishy. What's wrong with static inline int ?

 +{
 + int i;
 +
 + for (i = 0; i  cb-int_max; i++) {
 + if (cb-irq_map[i] == IRQ_FREE) {
 + cb-irq_map[i] = cb_no;
 + return i;
 + }
 + }
 +
 + return -ENODEV;
 +}

 +static int crossbar_domain_xlate(struct irq_domain *d,
 +  struct device_node *controller,
 +  const u32 *intspec, unsigned int intsize,
 +  unsigned long *out_hwirq,
 +  unsigned int *out_type)
 +{
 + unsigned long ret = 0;

Why do you need to initialize ret when you assign a value to it in the
next line?

Thanks,

tglx
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html