Re: [PATCH v3 6/9] irqchip: Add the ingenic-tcu-intc driver

[Paul Cercueil]

Hi Marc,

> >  +static int __init ingenic_tcu_intc_of_init(struct device_node 
> > *node,
> >  +	struct device_node *parent)
> >  +{
> >  +	struct irq_domain *domain;
> >  +	struct irq_chip_generic *gc;
> >  +	struct irq_chip_type *ct;
> >  +	int err, i, num_parent_irqs;
> >  +	unsigned int parent_irqs[3];
>
> 3 parent interrupts? Really? How do you pick one? Also, given the 
> useage
> model below, "int" is the wrong type. Probably should be u32.

See below.

> >  +	struct regmap *map;
> >  +
> >  +	num_parent_irqs = of_property_count_elems_of_size(
> >  +			node, "interrupts", 4);
>
> Nit: on a single line, as here is nothing that hurts my eyes more than
> reading something like(
> this). Also, 4 is better expressed as sizeof(u32).

That will make checkpatch.pl unhappy :(

> >  +	if (num_parent_irqs < 0 || num_parent_irqs > 
> > ARRAY_SIZE(parent_irqs))
> >  +		return -EINVAL;
> >  +
> >  +	map = syscon_node_to_regmap(node->parent);
> >  +	if (IS_ERR(map))
> >  +		return PTR_ERR(map);
> >  +
> >  +	domain = irq_domain_add_linear(node, 32, &irq_generic_chip_ops, 
> > NULL);
> >  +	if (!domain)
> >  +		return -ENOMEM;
> >  +
> >  +	err = irq_alloc_domain_generic_chips(domain, 32, 1, "TCU",
> >  +			handle_level_irq, 0, IRQ_NOPROBE | IRQ_LEVEL, 0);
> >  +	if (err)
> >  +		goto out_domain_remove;
> >  +
> >  +	gc = irq_get_domain_generic_chip(domain, 0);
> >  +	ct = gc->chip_types;
> >  +
> >  +	gc->wake_enabled = IRQ_MSK(32);
> >  +	gc->private = map;
> >  +
> >  +	ct->regs.disable = TCU_REG_TMSR;
> >  +	ct->regs.enable = TCU_REG_TMCR;
> >  +	ct->regs.ack = TCU_REG_TFCR;
> >  +	ct->chip.irq_unmask = ingenic_tcu_gc_unmask_enable_reg;
> >  +	ct->chip.irq_mask = ingenic_tcu_gc_mask_disable_reg;
> >  +	ct->chip.irq_mask_ack = ingenic_tcu_gc_mask_disable_reg_and_ack;
> >  +	ct->chip.flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE;
> >  +
> >  +	/* Mask all IRQs by default */
> >  +	regmap_write(map, TCU_REG_TMSR, IRQ_MSK(32));
> >  +
> >  +	for (i = 0; i < num_parent_irqs; i++) {
> >  +		parent_irqs[i] = irq_of_parse_and_map(node, i);
> >  +		if (!parent_irqs[i]) {
> >  +			err = -EINVAL;
> >  +			goto out_unmap_irqs;
> >  +		}
> >  +
> >  +		irq_set_chained_handler_and_data(parent_irqs[i],
> >  +				ingenic_tcu_intc_cascade, domain);
> >  +	}
>
> I don't get the multiple parent irq at all. How are the source 
> interrupt
> routed to the various cascades?

On JZ4740, channels 0 and 1 have their own interrupt, and channels 2-7 
share
one interrupt line.
On JZ4770 and JZ4780, the OST (OS timer) channel has its own interrupt,
channel 5 has its own interrupt, and channels 0-4 and 6-7 share one
interrupt line.

To keep things simple, we register the same interrupt handler for the 
three
parent interrupts.

-Paul