Re: [PATCH] ARM: mmp: parse irq from DT

On Fri, Jul 08, 2011 at 06:20:19PM +0800, Haojian Zhuang wrote:

for_each_compatible_node()

+		if (of_get_property(np, "interrupt-controller", NULL) == NULL)
+			continue;
+		nr = of_get_property(np, "sub-interrupts", NULL);
+		if (nr == NULL) {
+			printk(KERN_WARNING "sub-interrupts property "
+				"is missed\n");
+			continue;
+		}
+		irq_nr = be32_to_cpu(*nr);

There's a new function you can use here that makes reading single property
values easier.  of_platform_read_u32() is queued up in linux-next.

+		status = of_get_property(np, "status-mask", NULL);
+		if (status == NULL) {
+			printk(KERN_WARNING "status-mask property is missed\n");
+			continue;
+		}
+		edge = of_get_property(np, "mfp-edge-interrupt", NULL);
+
+		mux_info = kzalloc(sizeof(struct mmp_intc_info), GFP_KERNEL);

kzalloc(sizeof(*mux_info), GFP_KERNEL);

+		if (mux_info == NULL)
+			goto out;
+		mux_info->virt_base = mmp_info->virt_base;
+		mux_info->mux_status = be32_to_cpu(*status++)
+					+ mux_info->virt_base;
+		mux_info->mux_mask = be32_to_cpu(*status)
+					+ mux_info->virt_base;
+		if (edge) {
+			addr = be32_to_cpu(*edge) & PAGE_MASK;
+			offs = be32_to_cpu(*edge) - addr;
+			mux_info->mfp_edge = ioremap(addr, PAGE_SIZE) + offs;
+			mux_info->mfp_edge_index = be32_to_cpu(*++edge);
+		}
+
+		/* allocate new irq */
+		cascade = irq_of_parse_and_map(np, 0);
+		mux_info->irq_base = irq_alloc_descs(-1, 0, irq_nr, 0);
+		irq_domain_add_simple(np, mux_info->irq_base);
+
+		i = mux_info->irq_base;
+		for (; i < mux_info->irq_base + irq_nr; i++) {
+			irq_set_chip_data(i, mux_info);
+			mux_irq_mask(irq_get_irq_data(i));
+			irq_set_chip_and_handler(i, &mux_irq_chip,
+					handle_level_irq);
+			set_irq_flags(i, IRQF_VALID);
+		}
+
+		irq_set_handler_data(cascade, mux_info);
+		irq_set_chained_handler(cascade, mmp_irq_demux_handler);
+	}
+out:
+	return;
+}

It looks like this function is parsing a new device tree binding.  Any
new bindings needs to be documented in Documentation/devicetree/bindings.

+
+static void mmp_irq_unmask(struct irq_data *d)
+{
+	struct mmp_intc_info *info = irq_data_get_irq_chip_data(d);
+
+	/* ICU_INT_CONF */
+	__raw_writel(info->mask, info->virt_base + (d->irq << 2));
+}
+
+static void mmp_irq_mask(struct irq_data *d)
+{
+	struct mmp_intc_info *info = irq_data_get_irq_chip_data(d);
+
+	__raw_writel(0, info->virt_base + (d->irq << 2));
+}
+
+static struct irq_chip mmp_irq_chip = {
+	.name		= "mmp-intc",
+	.irq_unmask	= mmp_irq_unmask,
+	.irq_mask	= mmp_irq_mask,
+	.irq_ack	= mmp_irq_mask,
+};

Can you use IRQ_GENERIC_CHIP?

+
+void __init mmp_init_intc(void)
+{
+	struct mmp_intc_info *info;
+	struct device_node *np;
+	const __be32 *enable_mask, *base, *cell, *nr;
+	int i, irq_nr, phy_base;
+
+	np = of_find_compatible_node(NULL, NULL, "mrvl,mmp-intc");
+
+	BUG_ON(!np);
+
+	of_node_get(np);
+	if (of_get_property(np, "interrupt-controller", NULL) == NULL)
+		goto out;
+	cell = of_get_property(np, "#interrupt-cells", NULL);
+	if (cell == NULL) {
+		printk(KERN_ERR "mmp-intc: Device node %s missing "
+			"interrupt-cells property\n", np->full_name);
+		goto out;
+	}
+	if (be32_to_cpu(*cell) != 1) {
+		printk(KERN_ERR "mmp-intc: interrupt-cells property is "
+			"incorrect:%d\n", be32_to_cpu(*cell));
+		goto out;
+	}
+
+	nr = of_get_property(np, "sub-interrupts", NULL);
+	if (nr == NULL) {
+		printk(KERN_ERR "mmp-intc: interrupts property is missed\n");
+		goto out;
+	}
+	irq_nr = be32_to_cpu(*nr);
+
+	base = of_get_property(np, "reg", NULL);
+	if (base == NULL) {
+		printk(KERN_ERR "intc: Device node %s missing reg property\n",
+			np->full_name);
+		goto out;
+	}
+	phy_base = of_translate_address(np, base);

Use of_address_to_resource().

+
+	info = kzalloc(sizeof(struct mmp_intc_info), GFP_KERNEL);
+	if (info == NULL)
+		goto out;
+
+	enable_mask = of_get_property(np, "enable_mask", NULL);
+	if (enable_mask == NULL) {
+		printk(KERN_ERR "interrupt controller: Device node %s "
+			"missing interrupt property\n", np->full_name);
+		goto out_mem;
+	}
+	info->mask = be32_to_cpu(*enable_mask);
+
+	/* phy_base: 0, phy_size:64 */
+	info->phy_base = phy_base;
+	info->virt_base = ioremap(info->phy_base, PAGE_SIZE);
+
+	/* allocate new irq */
+	info->irq_base = irq_alloc_descs(-1, 0, irq_nr, 0);
+	irq_domain_add_simple(np, 0);

Excellent! Good use of dynamic irqs.

+
+	for (i = info->irq_base; i < info->irq_base + irq_nr; i++) {
+		irq_set_chip_data(i, info);
+		mmp_irq_mask(irq_get_irq_data(i));
+		irq_set_chip_and_handler(i, &mmp_irq_chip, handle_level_irq);
+		set_irq_flags(i, IRQF_VALID);
+	}
+	mux_init_intc(info);
+	of_node_put(np);
+	return;
+out_mem:
+	kfree(info);
+out:
+	of_node_put(np);
+	return;
+}

There's a lot of duplicate code here between the primary and cascaded
irq controllers.  It can probably be consolidated.

g.