[PATCH v3 1/3] ARM: sun7i/sun6i: irqchip: Add irqchip driver for NMI controller
Maxime Ripard
maxime.ripard at free-electrons.com
Tue Jan 28 05:40:44 EST 2014
On Fri, Jan 17, 2014 at 09:54:55AM +0100, Carlo Caione wrote:
> >> +/*
> >> + * Ack level interrupts right before unmask
> >> + *
> >> + * In case of level-triggered interrupt, IRQ line must be acked before it
> >> + * is unmasked or else a double-interrupt is triggered
> >> + */
> >> +
> >> +static void sunxi_sc_nmi_ack_and_unmask(struct irq_data *d)
> >> +{
> >> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> >> + struct irq_chip_type *ct = irq_data_get_chip_type(d);
> >> + u32 mask = d->mask;
> >> +
> >> + if (irqd_get_trigger_type(d) & IRQ_TYPE_LEVEL_MASK)
> >> + ct->chip.irq_ack(d);
> >> +
> >> + irq_gc_lock(gc);
> >> + irq_reg_writel(mask, gc->reg_base + ct->regs.mask);
> >> + irq_gc_unlock(gc);
> >> +}
> >
> > Hmmm, handle_level_irq seems to be doing exactly that already. It
> > first masks and acks the interrupts, and then unmask it, so we should
> > be fine, don't we?
>
> We don't, or at least handle_level_irq doesn't work for all the cases.
This is what I find weird :)
> Let's say (i.e. this is the cubieboard2 case) that to the irqchip is
> connected to the IRQ line of a PMIC accessed by I2C. In this case we
> cannot mask/ack the interrupt on the originating device in the hard
> interrupt handler (in which handle_level_irq is) since we need to
> access the I2C bus in an non-interrupt context.
We agree here.
> ACKing the IRQ in handle_level_irq at this point is pretty much
> useless since we still have to ACK the IRQs on the originating
> device and this will be done in a IRQ thread started after the hard
> IRQ handler.
Ok, so what you're saying is that you want to adress this case:
handle_level_irq
| device device
| mask ack handler irq acked unmask
| | | | | |
v v v v v v
NMI -> GIC:
+--------+ +-----------------
---------------+ +-----+
PMIC -> NMI:
+-+ +-+
------------+ +-----------+ +--------------------
Right?
But in this case, you would have two events coming from your device
(the two rising edges), so you'd expect two interrupts. And in the
case of a level triggered interrupt, the device would keep the
interrupt line active until it's unmasked, so we wouldn't end up with
this either.
> sunxi_sc_nmi_ack_and_unmask is therefore called (by
> irq_finalize_oneshot) after the IRQ thread has been executed. After
> the IRQ thread has ACKed the IRQs on the originating device we can
> finally ACK and unmask again the NMI.
And what happens if you get a new interrupt right between the end of
the handler and the unmask?
>
> >> +static inline void sunxi_sc_nmi_write(struct irq_chip_generic *gc, u32 off,
> >> + u32 val)
> >> +{
> >> + irq_reg_writel(val, gc->reg_base + off);
> >> +}
> >> +
> >> +static inline u32 sunxi_sc_nmi_read(struct irq_chip_generic *gc, u32 off)
> >> +{
> >> + return irq_reg_readl(gc->reg_base + off);
> >> +}
> >> +
> >> +static void sunxi_sc_nmi_handle_irq(unsigned int irq, struct irq_desc *desc)
> >> +{
> >> + struct irq_domain *domain = irq_desc_get_handler_data(desc);
> >> + struct irq_chip *chip = irq_get_chip(irq);
> >> + unsigned int virq = irq_find_mapping(domain, 0);
> >> +
> >> + chained_irq_enter(chip, desc);
> >> + generic_handle_irq(virq);
> >> + chained_irq_exit(chip, desc);
> >> +}
> >> +
> >> +static int sunxi_sc_nmi_set_type(struct irq_data *data, unsigned int flow_type)
> >> +{
> >> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
> >> + struct irq_chip_type *ct = gc->chip_types;
> >> + u32 src_type_reg;
> >> + u32 ctrl_off = ct->regs.type;
> >> + unsigned int src_type;
> >> + unsigned int i;
> >> +
> >> + irq_gc_lock(gc);
> >> +
> >> + switch (flow_type & IRQF_TRIGGER_MASK) {
> >> + case IRQ_TYPE_EDGE_FALLING:
> >> + src_type = SUNXI_SRC_TYPE_EDGE_FALLING;
> >> + break;
> >> + case IRQ_TYPE_EDGE_RISING:
> >> + src_type = SUNXI_SRC_TYPE_EDGE_RISING;
> >> + break;
> >> + case IRQ_TYPE_LEVEL_HIGH:
> >> + src_type = SUNXI_SRC_TYPE_LEVEL_HIGH;
> >> + break;
> >> + case IRQ_TYPE_NONE:
> >> + case IRQ_TYPE_LEVEL_LOW:
> >> + src_type = SUNXI_SRC_TYPE_LEVEL_LOW;
> >> + break;
> >> + default:
> >> + irq_gc_unlock(gc);
> >> + pr_err("%s: Cannot assign multiple trigger modes to IRQ %d.\n",
> >> + __func__, data->irq);
> >> + return -EBADR;
> >> + }
> >> +
> >> + irqd_set_trigger_type(data, flow_type);
> >> + irq_setup_alt_chip(data, flow_type);
> >> +
> >> + for (i = 0; i <= gc->num_ct; i++, ct++)
> >> + if (ct->type & flow_type)
> >> + ctrl_off = ct->regs.type;
> >> +
> >> + src_type_reg = sunxi_sc_nmi_read(gc, ctrl_off);
> >> + src_type_reg &= ~SUNXI_NMI_SRC_TYPE_MASK;
> >> + src_type_reg |= src_type;
> >> + sunxi_sc_nmi_write(gc, ctrl_off, src_type_reg);
> >> +
> >> + irq_gc_unlock(gc);
> >> +
> >> + return IRQ_SET_MASK_OK;
> >> +}
> >> +
> >> +static int __init sunxi_sc_nmi_irq_init(struct device_node *node,
> >> + struct sunxi_sc_nmi_reg_offs *reg_offs)
> >> +{
> >> + struct irq_domain *domain;
> >> + struct irq_chip_generic *gc;
> >> + unsigned int irq;
> >> + unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
> >> + int ret;
> >> +
> >> +
> >> + domain = irq_domain_add_linear(node, 1, &irq_generic_chip_ops, NULL);
> >> + if (!domain) {
> >> + pr_err("%s: Could not register interrupt domain.\n", node->name);
> >> + return -ENOMEM;
> >> + }
> >> +
> >> + ret = irq_alloc_domain_generic_chips(domain, 1, 2, node->name,
> >> + handle_level_irq, clr, 0,
> >> + IRQ_GC_INIT_MASK_CACHE);
> >> + if (ret) {
> >> + pr_err("%s: Could not allocate generic interrupt chip.\n",
> >> + node->name);
> >> + goto fail_irqd_remove;
> >> + }
> >> +
> >> + irq = irq_of_parse_and_map(node, 0);
> >> + if (irq <= 0) {
> >> + pr_err("%s: unable to parse irq\n", node->name);
> >> + ret = -EINVAL;
> >> + goto fail_irqd_remove;
> >> + }
> >> +
> >> + gc = irq_get_domain_generic_chip(domain, 0);
> >> + gc->reg_base = of_iomap(node, 0);
> >> + if (!gc->reg_base) {
> >> + pr_err("%s: unable to map resource\n", node->name);
> >> + ret = -ENOMEM;
> >> + goto fail_irqd_remove;
> >> + }
> >> +
> >> + gc->chip_types[0].type = IRQ_TYPE_LEVEL_MASK;
> >> + gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
> >> + gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
> >> + gc->chip_types[0].chip.irq_unmask = sunxi_sc_nmi_ack_and_unmask;
> >> + gc->chip_types[0].chip.irq_set_type = sunxi_sc_nmi_set_type;
> >> + gc->chip_types[0].regs.ack = reg_offs->pend;
> >> + gc->chip_types[0].regs.mask = reg_offs->enable;
> >> + gc->chip_types[0].regs.type = reg_offs->ctrl;
> >> +
> >> + gc->chip_types[1].type = IRQ_TYPE_EDGE_BOTH;
> >> + gc->chip_types[1].chip.name = gc->chip_types[0].chip.name;
> >> + gc->chip_types[1].chip.irq_ack = irq_gc_ack_set_bit;
> >> + gc->chip_types[1].chip.irq_mask = irq_gc_mask_clr_bit;
> >> + gc->chip_types[1].chip.irq_unmask = sunxi_sc_nmi_ack_and_unmask;
> >> + gc->chip_types[1].chip.irq_set_type = sunxi_sc_nmi_set_type;
> >> + gc->chip_types[1].regs.ack = reg_offs->pend;
> >> + gc->chip_types[1].regs.mask = reg_offs->enable;
> >> + gc->chip_types[1].regs.type = reg_offs->ctrl;
> >> + gc->chip_types[1].handler = handle_edge_irq;
> >> +
> >> + irq_set_handler_data(irq, domain);
> >> + irq_set_chained_handler(irq, sunxi_sc_nmi_handle_irq);
> >> +
> >> + sunxi_sc_nmi_write(gc, reg_offs->enable, 0);
> >> + sunxi_sc_nmi_write(gc, reg_offs->pend, 0x1);
> >
> > I really wonder whether it makes sense to have a generic chip here. It
> > seems to be much more complicated than it should. It's only about a
> > single interrupt interrupt chip here.
>
> I agree but is there any other way to manage the NMI without the
> driver of the device connected to NMI having to worry about
> acking/masking/unmasking/ etc..?
Yes, just declare a "raw" irqchip. Pretty much like we do on irq-sun4i
for example.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/416961c6/attachment.sig>
More information about the linux-arm-kernel
mailing list