Re: [PATCH v2] drivers: irqchip: add irq-type-changer

From: Nikita Yushchenko
Date: Tue Jan 25 2022 - 04:39:59 EST


I also don't see why you model this as the actual device that triggers
the interrupt.

Well, that somehow matches the physical reality. In the case of wl18xx
on KF, physically the interrupt signal indeed originates from the
intermediate chip - the inverting level-shifter.

Reality? By allowing something like an edge-to-level conversion? How
can that work?

Edge to level can be problematic, but level to edge does not cause any difficulties, nor in generating nor in handling.

In contrary, an abstraction of intermediate entity that does a static
conversion of the trigger type and does not need any software control,
looks sane. And, hardware designers do strange things sometimes, I
won't be surprised observing a change from level to edge one day.

If you think that it can happen without a HW register that latches the
edge and requires an ack, you need to question your understanding of
an interrupt life cycle, and of the properties of the various trigger
types.

There are plenty of devices capable to signal both level and edge interrupts, configurable by a register. Basic handling is always the same, and involves masking the interrupt at interrupt controller while IRQs disabled, then enabling IRQs, then programming the device to clear the interrupt condition, then unmasking the interrupt at interrupt controller. If the trigger type is level or edge, is only interesting to interrupt controller driver, but not to a wider scope.

Nothing stops hw designers from doing all sort of strange things with interrupt signals. Right now I have a board on my desk where interrupt signals from several chips are wired to inputs of a logical AND element and the output of that is wired to SoC's gpio. I don't see what stops them tomorrow from setting up their CPLDs to issue a short impulse at output in return to a level change on input. And that will be a level to edge converter.

In driver's init, I only call of_irq_parse_one() for interrupt defined
in the changer's node. This does not create a mapping for it. The
mapping is only created when changer's "interrupt-child" creates a
mapping for their interrupt - then changer's alloc() routine calls
irq_domain_alloc_irqs_parent() in the same way as all other
hierarchical irqchips do.

I don't see where double mapping can appear here. Please explain.

Just look at your code. You start probing a device that has an
'interrupts' property. This triggers the allocation of an interrupt.

Where does it trigger it?

And what is this "allocation", after all? Is it allocation of virq number? That allocation happens when irq_create_fwspec_mapping() calls irq_domain_alloc_irqs(). But this path does not necessary gets executed in probing. In the current irq_tyoe_changer driver, it is not.


Nikita