Re: [PATCH 1/2] versatile: sic: add device tree bindings

On Fri, Jan 13, 2012 at 11:41:51AM -0700, Grant Likely wrote:

On Fri, Jan 13, 2012 at 10:58:34AM +0000, Jamie Iles wrote:

quoted

On Fri, Jan 13, 2012 at 10:48:42AM +0000, Russell King - ARM Linux wrote:

quoted

On Fri, Jan 13, 2012 at 12:44:00AM +0000, Jamie Iles wrote:

quoted

+#ifdef CONFIG_OF
+int __init sic_of_init(struct device_node *np, struct device_node *parent)
+{
+	struct fpga_irq_data *sic_data = kzalloc(sizeof(*sic_data), GFP_KERNEL);
+	int err = -ENOMEM, irq;
+
+	if (WARN_ON(!sic_data))
+		return err;
+
+	sic_data->base = of_iomap(np, 0);
+	if (WARN_ON(!sic_data->base))
+		goto out_free_data;
+
+	irq = irq_of_parse_and_map(np, 0);
+	if (irq < 0)
+		goto out_free_data;
+
+	sic_data->irq_start = irq_alloc_descs(-1, 0, 32, numa_node_id());
+	if (WARN_ON(sic_data->irq_start < 0)) {
+		err = sic_data->irq_start;
+		goto out_unmap;
+	}
+	sic_data->domain.of_node = of_node_get(np);
+
+	fpga_irq_init(irq, ~0, sic_data);

I notice that you completely ignore the validity mask, and initialize all
32 interrupts.  This is wrong.  You don't think I put the validity mask
there just to obfuscate the code?

OK, that is wrong.  The validity mask is used for the non-DT platforms, 
for the DT one I'll add a property stating the valid mask:

	arm,valid-irq-mask = <0x001fffff>;

to the node in the trees.

Shouldn't be necessary when the driver is converted to using the
linear mapping.  When that happens, only the requested irqs will get
mapped.

What I'm saying is that they shouldn't even be _published_ as
potentially valid IRQs.